From: | Merlin Moncure <mmoncure(at)gmail(dot)com> |
---|---|
To: | Andres Freund <andres(at)2ndquadrant(dot)com> |
Cc: | Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: StrategyGetBuffer optimization, take 2 |
Date: | 2013-08-22 13:07:44 |
Message-ID: | CAHyXU0xB2khwv3E8gTjfpmbYdVhOc1V3DE0jwHAbg5aOy5USog@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Aug 20, 2013 at 1:57 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2013-08-19 15:17:44 -0700, Jeff Janes wrote:
>> On Wed, Aug 7, 2013 at 7:40 AM, Merlin Moncure <mmoncure(at)gmail(dot)com> wrote:
>>
>> > I agree; at least then it's not unambiguously better. if you (in
>> > effect) swap all contention on allocation from a lwlock to a spinlock
>> > it's not clear if you're improving things; it would have to be proven
>> > and I'm trying to keep things simple.
>> >
>> > Attached is a scaled down version of the patch that keeps the freelist
>> > lock but still removes the spinlock during the clock sweep. This
>> > still hits the major objectives of reducing the chance of scheduling
>> > out while holding the BufFreelistLock and mitigating the worst case
>> > impact of doing so if it does happen. An even more scaled down
>> > version would keep the current logic exactly as is except for
>> > replacing buffer lock in the clock sweep with a trylock (which is
>> > IMNSHO a no-brainer).
>>
>> Since usage_count is unsigned, are you sure that changing the tests
>> from "buf->usage_count == 0" to "buf->usage_count <= 0" accomplishes
>> what you need it to? If usage_count gets decremented when it already
>> zero, it will wrap around to 65,535, at least on some compilers some
>> of the time, won't it?
>
> Overflow of *unsigned* variables is actually defined and will always
> wrap around. It's signed variables which don't have such a clear
> behaviour.
Hm, well, even better would be to leave things as they are and try to
guarantee that usage_count is updated via assignment vs increment;
that way it would be impossible to wander out of bounds. I bet
changing:
i--; to i=(i-1);
isn't going to do much against modern compilers. But what about
assignment from a volatile temporary?
volatile v = usage_count;
if (v > 0) v--;
usage_count = v;
something like that. Or maybe declaring usage_count as volatile might
be enough?
merlin
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2013-08-22 13:21:28 | Re: pg_system_identifier() |
Previous Message | Vik Fearing | 2013-08-22 12:53:47 | Re: pg_system_identifier() |