Hi, GetSnapshotData() takes the ProcArrayLock in read-only mode for several reasons:
1. To acquire new snapshot data; 2. To make sure that no active transaction commits before we install the new snapshot data's xmin, if we didn't already have an xmin installed; and 3. To validate that the snapshot it currently holds hasn't been invalidated by a newly committed transaction, by comparing snapshot->transXactCompletionCount against TransamVariables->xactCompletionCount. I noticed [0] that the reuse path could be implemented as just an atomic read (thus avoiding touching the cache line that backs ProcArrayLock) if these conditions hold: 1. We have a cached snapshot that we'd like to reuse; 2. Our backend already has its MyProc->xmin installed; and 3. The output of an atomic read of TransamVariables->xactCompletionCount indicates its value hasn't changed since the to-be-reused snapshot was taken. Actually updating the snapshot contents would still require taking the lock, and likewise would installing MyProc->xmin, but for some workloads this'll probably save a lot of RW traffic on the ProcArrayLock cache line. Data collected from running the test suite with some instrumentation (in 0002-nocfbot.patch) indicates ~50% of GetSnapshotData() calls benefit from this unlocked GetSnapshotDataReuse optimization: 1174391 of 2288104 calls to GetSnapshotData used the new unlocked path, with 866907 (37%) not taking the path because it would need to install an xmin, and 246134 (11%) failing on the xactCompletionCount check and needing to update the snapshot, whilst the remaining 672 (0.03%) of the sessions didn't have a cached snapshot to reuse. I've attached a patch that applies this optimization. The patch is quite a bit larger larger than I'd hoped, because adding port/atomics.h to access/transam.h causes frontend compilation errors. This effectively required me to move TransamVariables into a different header, this case a new varsup.h, which we then need to included in many sources, which increases the size of the patch. Thoughts? Kind regards, Matthias van de Meent Databricks (https://www.databricks.com) [0] after implementing this, I looked at commit logs for the Reuse path, it looks like the original xactCompletionCount commit 623a9ba79b already hinted at this optimization in its message as being a likely possible further optimization. However, nobody seems to have since gotten to actually implementing it.
v1-0002-nocfbot.patch
Description: Binary data
v1-0001-Draft-Unlocked-path-for-GetSnapshotDataReuse.patch
Description: Binary data
