On Tue, Oct 14, 2014 at 8:58 AM, Andres Freund <and...@2ndquadrant.com> wrote: > On 2014-10-14 08:40:49 -0500, Merlin Moncure wrote: >> On Fri, Oct 10, 2014 at 11:00 AM, Andres Freund <and...@2ndquadrant.com> >> wrote: >> > Which is nearly trivial now that atomics are in. Check out the attached >> > WIP patch which eliminates the spinlock from StrategyGetBuffer() unless >> > there's buffers on the freelist. >> >> Is this safe? >> >> + victim = pg_atomic_fetch_add_u32(&StrategyControl->nextVictimBuffer, 1); >> >> - if (++StrategyControl->nextVictimBuffer >= NBuffers) >> + buf = &BufferDescriptors[victim % NBuffers]; >> + >> + if (victim % NBuffers == 0) >> <snip> >> >> I don't think there's any guarantee you could keep nextVictimBuffer >> from wandering off the end. > > Not sure what you mean? It'll cycle around at 2^32. The code doesn't try > to avoid nextVictimBuffer from going above NBuffers. To avoid overrunning > &BufferDescriptors I'm doing % NBuffers. > > Yes, that'll have the disadvantage of the first buffers being slightly > more likely to be hit, but for that to be relevant you'd need > unrealistically large amounts of shared_buffers.
Right -- my mistake. That's clever. I think that should work well. > I think that's pretty much orthogonal. I *do* think it makes sense to > increment nextVictimBuffer in bigger steps. But the above doesn't > prohibit doing so - and it'll still be be much better without the > spinlock. Same number of atomics, but no potential of spinning and no > potential of being put to sleep while holding the spinlock. > > This callsite has a comparatively large likelihood of being put to sleep > while holding a spinlock because it can run for a very long time doing > nothing but spinlocking. A while back, I submitted a minor tweak to the clock sweep so that, instead of spinlocking every single buffer header as it swept it just did a single TAS as a kind of a trylock and punted to the next buffer if the test failed on the principle there's not good reason to hang around. You only spin if you passed the first test; that should reduce the likelihood of actual spinning to approximately zero. I still maintain there's no reason not to do that (I couldn't show a benefit but that was because mapping list locking was masking any clock sweep contention at that time). merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers