From: | Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru> |
---|---|
To: | "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | sinvaladt.c: remove msgnumLock, use atomic operations on maxMsgNum |
Date: | 2025-02-03 12:05:30 |
Message-ID: | 30aa0030-f694-44ef-a19d-6ef7ddb69374@postgrespro.ru |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Good day,
Investigating some performance issues of a client, our engineers found
msgnumLock to be contended.
Looking closer it is obvious it is not needed at all: it used only as
memory barrier. It is even stated in comment at file start:
* We deal with that by having a spinlock that readers must take for just
* long enough to read maxMsgNum, while writers take it for just long enough
* to write maxMsgNum. (The exact rule is that you need the spinlock to
* read maxMsgNum if you are not holding SInvalWriteLock, and you need the
* spinlock to write maxMsgNum unless you are holding both locks.)
*
* Note: since maxMsgNum is an int and hence presumably atomically readable/
* writable, the spinlock might seem unnecessary. The reason it is needed
* is to provide a memory barrier: we need to be sure that messages written
* to the array are actually there before maxMsgNum is increased, and that
* readers will see that data after fetching maxMsgNum.
So we changed maxMsgNum to be pg_atomic_uint32, and put appropriate memory
barriers:
- pg_write_barrier() before write to maxMsgNum (when only SInvalWriteLock
is held)
- pg_read_barrier() after read of maxMsgNum (when only SInvalReadLock is held)
It improved situation for our client.
Note: pg_(write|read)_barrier() is chosen instead of
pg_atomic_(read|write)_membarrier_u32() because it certainly cheaper at
least on x86_64 where it is translated to just a compiler barrier (empty
asm statement).
At least pg_atomic_read_membarrier_u32() is implemented currently as a
write operation, that's not good for contended place.
-----
regards
Yura Sokolov aka funny-falcon
Attachment | Content-Type | Size |
---|---|---|
v0-0001-sinvaladt.c-use-atomic-operations-on-maxMsgNum.patch | text/x-patch | 6.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Pavel Borisov | 2025-02-03 12:16:03 | Re: POC, WIP: OR-clause support for indexes |
Previous Message | Nisha Moond | 2025-02-03 12:05:22 | Re: Introduce XID age and inactive timeout based replication slot invalidation |