Re: StrategyGetBuffer optimization, take 2

On Wed, Aug 7, 2013 at 10:37 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2013-08-07 09:40:24 -0500, Merlin Moncure wrote:
>> > I don't think the unlocked increment of nextVictimBuffer is a good idea
>> > though. nextVictimBuffer jumping over NBuffers under concurrency seems
>> > like a recipe for disaster to me. At the very, very least it will need a
>> > good wad of comments explaining what it means and how you're allowed to
>> > use it. The current way will lead to at least bgwriter accessing a
>> > nonexistant/out of bounds buffer via StrategySyncStart().
>> > Possibly it won't even save that much, it might just increase the
>> > contention on the buffer header spinlock's cacheline.
>>
>> 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.
>
> I think converting it to a spinlock actually is a good idea, you just
> need to expand the scope a bit.

Umm, sorry if I am being naive, but dont spinlocks perform bad when a
lot of contention is present on that node?

I feel that we may hit on that case here. A preliminary check before
the actual spinlock may be good,though,since spinlocks are cheap until
the contention remains low.

Regards,

Atri

--
Regards,

Atri
l'apprenant