From 11e178d0dc4bc2328ae4759090b3c48b07023fab Mon Sep 17 00:00:00 2001
From: Kevin Grittner <kgrittn@postgresql.org>
Date: Thu, 21 Apr 2016 08:40:08 -0500
Subject: [PATCH] Inline initial comparisons in TestForOldSnapshot()

Even with old_snapshot_threshold = -1 (which disables the "snapshot
too old" feature), performance regressions were seen at moderate to
high concurrency.  For example, a one-socket, four-core system
running 200 connections at saturation could see up to a 2.3%
regression, with larger regressions possible on NUMA machines.
By inlining the early (smaller, faster) tests in the
TestForOldSnapshot() function, the i7 case dropped to a 0.2%
regression, which could easily just be noise, and is clearly an
improvement.  Further testing will show whether more is needed.
---
 src/backend/storage/buffer/bufmgr.c | 28 +++++--------------------
 src/include/storage/bufmgr.h        | 32 ++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index ac1e513560..c0d5fa74e1 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -4282,33 +4282,15 @@ IssuePendingWritebacks(WritebackContext *context)
 
 
 /*
- * Check whether the given snapshot is too old to have safely read the given
- * page from the given table.  If so, throw a "snapshot too old" error.
+ * Implement slower/larger portions of TestForOldSnapshot
  *
- * This test generally needs to be performed after every BufferGetPage() call
- * that is executed as part of a scan.  It is not needed for calls made for
- * modifying the page (for example, to position to the right place to insert a
- * new index tuple or for vacuuming).  It may also be omitted where calls to
- * lower-level functions will have already performed the test.
- *
- * Note that a NULL snapshot argument is allowed and causes a fast return
- * without error; this is to support call sites which can be called from
- * either scans or index modification areas.
- *
- * For best performance, keep the tests that are fastest and/or most likely to
- * exclude a page from old snapshot testing near the front.
+ * Smaller/faster portions are put inline, but the entire set of logic is too
+ * big for that.
  */
 void
-TestForOldSnapshot(Snapshot snapshot, Relation relation, Page page)
+TestForOldSnapshot_impl(Snapshot snapshot, Relation relation)
 {
-	Assert(relation != NULL);
-
-	if (old_snapshot_threshold >= 0
-	 && (snapshot) != NULL
-	 && (snapshot)->satisfies == HeapTupleSatisfiesMVCC
-	 && !XLogRecPtrIsInvalid((snapshot)->lsn)
-	 && PageGetLSN(page) > (snapshot)->lsn
-	 && !IsCatalogRelation(relation)
+	if (!IsCatalogRelation(relation)
 	 && !RelationIsAccessibleInLogicalDecoding(relation)
 	 && (snapshot)->whenTaken < GetOldSnapshotThresholdTimestamp())
 		ereport(ERROR,
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index b514b4696d..38b602724e 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -239,7 +239,7 @@ extern bool BgBufferSync(struct WritebackContext *wb_context);
 
 extern void AtProcExit_LocalBuffers(void);
 
-extern void TestForOldSnapshot(Snapshot snapshot, Relation relation, Page page);
+extern void TestForOldSnapshot_impl(Snapshot snapshot, Relation relation);
 
 /* in freelist.c */
 extern BufferAccessStrategy GetAccessStrategy(BufferAccessStrategyType btype);
@@ -257,6 +257,36 @@ extern void FreeAccessStrategy(BufferAccessStrategy strategy);
 
 #ifndef FRONTEND
 
+/*
+ * Check whether the given snapshot is too old to have safely read the given
+ * page from the given table.  If so, throw a "snapshot too old" error.
+ *
+ * This test generally needs to be performed after every BufferGetPage() call
+ * that is executed as part of a scan.  It is not needed for calls made for
+ * modifying the page (for example, to position to the right place to insert a
+ * new index tuple or for vacuuming).  It may also be omitted where calls to
+ * lower-level functions will have already performed the test.
+ *
+ * Note that a NULL snapshot argument is allowed and causes a fast return
+ * without error; this is to support call sites which can be called from
+ * either scans or index modification areas.
+ *
+ * For best performance, keep the tests that are fastest and/or most likely to
+ * exclude a page from old snapshot testing near the front.
+ */
+static inline void
+TestForOldSnapshot(Snapshot snapshot, Relation relation, Page page)
+{
+	Assert(relation != NULL);
+
+	if (old_snapshot_threshold >= 0
+		&& (snapshot) != NULL
+		&& (snapshot)->satisfies == HeapTupleSatisfiesMVCC
+		&& !XLogRecPtrIsInvalid((snapshot)->lsn)
+		&& PageGetLSN(page) > (snapshot)->lsn)
+		TestForOldSnapshot_impl(snapshot, relation);
+}
+
 #endif   /* FRONTEND */
 
 #endif   /* BUFMGR_H */