sinvaladt.c: remove msgnumLock, use atomic operations on maxMsgNum

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

Responses

Browse pgsql-hackers by date

  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