Re: Fix performance degradation of contended LWLock on NUMA

On 2017-10-20 11:54, Sokolov Yura wrote:
> Hello,
>
> On 2017-10-19 19:46, Andres Freund wrote:
>> On 2017-10-19 14:36:56 +0300, Sokolov Yura wrote:
>>> > > + init_local_spin_delay(&delayStatus);
>>> >
>>> > The way you moved this around has the disadvantage that we now do this -
>>> > a number of writes - even in the very common case where the lwlock can
>>> > be acquired directly.
>>>
>>> Excuse me, I don't understand fine.
>>> Do you complain against init_local_spin_delay placed here?
>>
>> Yes.
>
> I could place it before perform_spin_delay under `if (!spin_inited)` if
> you
> think it is absolutely must.

I looked at assembly, and remembered, that last commit simplifies
`init_local_spin_delay` to just two-three writes of zeroes (looks
like compiler combines 2*4byte write into 1*8 write). Compared to
code around (especially in LWLockAcquire itself), this overhead
is negligible.

Though, I found that there is benefit in calling LWLockAttemptLockOnce
before entering loop with calls to LWLockAttemptLockOrQueue in the
LWLockAcquire (in there is not much contention). And this way, `inline`
decorator for LWLockAttemptLockOrQueue could be omitted. Given, clang
doesn't want to inline this function, it could be the best way.

Should I add such commit to patch?

--
With regards,
Sokolov Yura aka funny_falcon
Postgres Professional: https://postgrespro.ru
The Russian Postgres Company