Re: Wait free LW_SHARED acquisition - v0.9

On Fri, Oct 10, 2014 at 11:00 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2014-10-10 16:41:39 +0200, Andres Freund wrote:
>> FWIW, the profile always looks like
>> - 48.61% postgres postgres [.] s_lock
>> - s_lock
>> + 96.67% StrategyGetBuffer
>> + 1.19% UnpinBuffer
>> + 0.90% PinBuffer
>> + 0.70% hash_search_with_hash_value
>> + 3.11% postgres postgres [.] GetSnapshotData
>> + 2.47% postgres postgres [.] StrategyGetBuffer
>> + 1.93% postgres [kernel.kallsyms] [k] copy_user_generic_string
>> + 1.28% postgres postgres [.] hash_search_with_hash_value
>> - 1.27% postgres postgres [.] LWLockAttemptLock
>> - LWLockAttemptLock
>> - 97.78% LWLockAcquire
>> + 38.76% ReadBuffer_common
>> + 28.62% _bt_getbuf
>> + 8.59% _bt_relandgetbuf
>> + 6.25% GetSnapshotData
>> + 5.93% VirtualXactLockTableInsert
>> + 3.95% VirtualXactLockTableCleanup
>> + 2.35% index_fetch_heap
>> + 1.66% StartBufferIO
>> + 1.56% LockReleaseAll
>> + 1.55% _bt_next
>> + 0.78% LockAcquireExtended
>> + 1.47% _bt_next
>> + 0.75% _bt_relandgetbuf
>>
>> to me. Now that's with the client count 496, but it's similar with lower
>> counts.
>>
>> BTW, that profile *clearly* indicates we should make StrategyGetBuffer()
>> smarter.
>
> 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. You could buy a little safety by CAS'ing
zero in instead, followed by atomic increment. However that's still
pretty dodgy IMO and requires two atomic ops which will underperform
the spin in some situations.

I like Robert's idea to keep the spinlock but preallocate a small
amount of buffers, say 8 or 16.

merlin