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