POC: Unlocked path for GetSnapshotDataReuse

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.