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