Re: Replace known_assigned_xids_lck by memory barrier
Michail Nikolaev <michail.nikolaev@gmail.com>
From: Michail Nikolaev <michail.nikolaev@gmail.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>,
Tom Lane <tgl@sss.pgh.pa.us>
Date: 2023-08-15T10:29:24Z
Lists: pgsql-hackers
Hello, Nathan. > What sort of benefits do you see from this patch? It might be worthwhile > in itself to remove spinlocks when possible, but IME it's much easier to > justify such changes when there is a tangible benefit we can point to. Oh, it is not an easy question :) The answer, probably, looks like this: 1) performance benefits of spin lock acquire removing in KnownAssignedXidsGetOldestXmin and KnownAssignedXidsSearch 2) it is closing 13-year-old tech depth But in reality, it is not easy to measure performance improvement consistently for this change. > Are the assignments in question guaranteed to be atomic? IIUC we assume > that aligned 4-byte loads/stores are atomic, so we should be okay as long > as we aren't handling anything larger. Yes, 4-bytes assignment are atomic, locking is used to ensure memory write ordering in this place. > This use of pg_write_barrier() looks correct to me, but don't we need > corresponding read barriers wherever we obtain the pointers? FWIW I tend > to review src/backend/storage/lmgr/README.barrier in its entirety whenever > I deal with this stuff. Oh, yeah, you're right! (1) I'll prepare an updated version of the patch soon. I don't why I was assuming pg_write_barrier is enough (⊙_⊙') [1]: https://github.com/postgres/postgres/blob/master/src/backend/storage/lmgr/README.barrier#L125
Commits
-
Replace known_assigned_xids_lck with memory barriers.
- 119c23eb9819 17.0 landed
-
Replace the KnownAssignedXids hash table with a sorted-array data structure,
- 2871b4618af1 9.0.0 cited