On Mon, May 08, 2023 at 08:18:04PM +0530, Bharath Rupireddy wrote: > On Mon, May 8, 2023 at 5:57 PM Bharath Rupireddy > <bharath.rupireddyforpostg...@gmail.com> wrote: >> On Mon, Apr 10, 2023 at 9:38 AM Michael Paquier <mich...@paquier.xyz> wrote: >>> The sensitive change is in LWLockUpdateVar(). I am not completely >>> sure to understand this removal, though. Does that influence the case >>> where there are waiters? >> >> I'll send about this in a follow-up email to not overload this >> response with too much data. > > It helps the case when there are no waiters. IOW, it updates value > without waitlist lock when there are no waiters, so no extra waitlist > lock acquisition/release just to update the value. In turn, it helps > the other backend wanting to flush the WAL looking for the new updated > value of insertingAt in WaitXLogInsertionsToFinish(), now the flushing > backend can get the new value faster.
Sure, which is what the memory barrier given by exchange_u64 guarantees. My thoughts on this one is that I am not completely sure to understand that we won't miss any waiters that should have been awaken. > The fastpath exit in LWLockUpdateVar() doesn't seem to influence the > results much, see below results. However, it avoids waitlist lock > acquisition when there are no waiters. > > test-case 1: -T5, WAL ~16 bytes > clients HEAD PATCHED with fastpath PATCHED no fast path > 64 50135 45528 46653 > 128 94903 89791 89103 > 256 82289 152915 152835 > 512 62498 138838 142084 > 768 57083 125074 126768 > 1024 51308 113593 115930 > 2048 41084 88764 85110 > 4096 19939 42257 43917 Considering that there could be a few percents of noise mixed into that, that's not really surprising as the workload is highly concurrent on inserts so the fast path won't really shine :) Should we split this patch into two parts, as they aim at tackling two different cases then? One for LWLockConflictsWithVar() and LWLockReleaseClearVar() which are the straight-forward pieces (using one pg_atomic_write_u64() in LWLockUpdateVar instead), then a second for LWLockUpdateVar()? Also, the fast path treatment in LWLockUpdateVar() may show some better benefits when there are really few backends and a bunch of very little records? Still, even that sounds a bit limited.. > Out of 3 functions that got rid of waitlist lock > LWLockConflictsWithVar/LWLockWaitForVar, LWLockUpdateVar, > LWLockReleaseClearVar, perf reports tell that the biggest gain (for > the use-cases that I've tried) is for > LWLockConflictsWithVar/LWLockWaitForVar: > > test-case 6: -T5, WAL 4096 bytes > HEAD: > + 29.66% 0.07% postgres [.] LWLockWaitForVar > + 20.94% 0.08% postgres [.] LWLockConflictsWithVar > 0.19% 0.03% postgres [.] LWLockUpdateVar > > PATCHED: > + 3.95% 0.08% postgres [.] LWLockWaitForVar > 0.19% 0.03% postgres [.] LWLockConflictsWithVar > + 1.73% 0.00% postgres [.] LWLockReleaseClearVar Indeed. -- Michael
signature.asc
Description: PGP signature
