atomic reads & writes (with no barriers)

The "snapshot too old" patch has an XXX comment about being able to
drop a spinlock usage from a frequently called "get" method if the
64-bit read could be trusted to be atomic. There is no need for a
barrier in this case, because a stale value just means we won't be
quite as aggressive about pruning tuples still within the global
xmin boundary (potentially leading to the "snapshot too old"
error), normally by a small fraction of a second. Removing
contention on spinlocks is a good thing. Andres commented here:

http://www.postgresql.org/message-id/20151203154230.GE20698@awork2.anarazel.de

| We currently don't assume it's atomic. And there are platforms,
| e.g 32 bit arm, where that's not the case (c.f.
| https://wiki.postgresql.org/wiki/Atomics ). It'd be rather useful
| to abstract that knowledge into a macro...

I did some web searches and then opened a question on Stack
Overflow. Not surprisingly I got a mix of useful answers, and
those not quite so much. Based on the most useful comment, I got
something that isn't too awfully pretty, but it seems to work on my
Ubuntu machine. Maybe it can be the start of something useful.

Before C11 the C standard specifically disclaimed any atomic
access. C11 adds what seems to me to be a fairly nice mechanism
for supporting it with the _Atomic modifier on a declaration.
Based on a suggestion from Ian Abbott I added this:

diff --git a/src/include/c.h b/src/include/c.h
index 8163b00..105c542 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -288,6 +288,11 @@ typedef unsigned long long int uint64;
#error must have a working 64-bit integer datatype
#endif

+#if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 201112L) &&
!defined(__STDC_NO_ATOMICS__)
+#define HAVE_ATOMICINT64
+typedef _Atomic int64 atomicint64;
+#endif
+
/* Decide if we need to decorate 64-bit constants */
#ifdef HAVE_LL_CONSTANTS
#define INT64CONST(x) ((int64) x##LL)

To use it in my patch, I made these changes to the patched code:

diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 13c58d2..1b891b5 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -81,7 +81,11 @@ typedef struct OldSnapshotControlData
slock_t mutex_current; /* protect current timestamp */
int64 current_timestamp; /* latest snapshot timestamp */
slock_t mutex_threshold; /* protect threshold fields */
+#ifdef HAVE_ATOMICINT64
+ atomicint64 threshold_timestamp; /* earlier snapshot is old */
+#else
int64 threshold_timestamp; /* earlier snapshot is old */
+#endif
TransactionId threshold_xid; /* earlier xid may be gone */

/*
@@ -1553,6 +1557,9 @@ GetSnapshotCurrentTimestamp(void)
int64
GetOldSnapshotThresholdTimestamp(void)
{
+#ifdef HAVE_ATOMICINT64
+ return oldSnapshotControl->threshold_timestamp;
+#else
int64 threshold_timestamp;

SpinLockAcquire(&oldSnapshotControl->mutex_threshold);
@@ -1560,6 +1567,7 @@ GetOldSnapshotThresholdTimestamp(void)
SpinLockRelease(&oldSnapshotControl->mutex_threshold);

return threshold_timestamp;
+#endif
}

/*

I'm sure this could be improved upon, and we would want to expand
it to uint64; but it seems to work, and I think it should do no
harm on non-C11 compilers. (I'm less sure about 32-bit builds, but
if there's a problem there, it should be fixable.) We may be able
to make use of non-standard features of other (non-C11) compilers,
but I'm not aware of what options there are.

Is the c.h change above on anything resembling the right track for
a patch for this? If not, what would such a patch look like?

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company