From 6bad580d9e678a0b604883e14d8401d469b06566 Mon Sep 17 00:00:00 2001
From: Simon Riggs <simon@2ndQuadrant.com>
Date: Thu, 6 Apr 2017 08:31:52 -0400
Subject: [PATCH] Avoid SnapshotResetXmin() during AtEOXact_Snapshot()

For normal commits and aborts we already reset PgXact->xmin,
so we can simply avoid running SnapshotResetXmin() twice.

During performance tests by Alexander Korotkov, diagnosis
by Andres Freund showed PgXact array as a bottleneck. After
manual analysis by me of the code paths that touch those
memory locations, I was able to identify extraneous code
in the main transaction commit path.

Avoiding touching highly contented shmem improves concurrent
performance slightly on all workloads, confirmed by tests
run by Ashutosh Sharma and Alexander Korotkov.

Simon Riggs

Discussion: CANP8+jJdXE9b+b9F8CQT-LuxxO0PBCB-SZFfMVAdp+akqo4zfg@mail.gmail.com
---
 src/backend/access/transam/xact.c |  6 +++---
 src/backend/utils/time/snapmgr.c  | 14 ++++++++++++--
 src/include/utils/snapmgr.h       |  2 +-
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 6f614e4fad..84409ea956 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -2137,7 +2137,7 @@ CommitTransaction(void)
 	AtEOXact_ComboCid();
 	AtEOXact_HashTables(true);
 	AtEOXact_PgStat(true);
-	AtEOXact_Snapshot(true);
+	AtEOXact_Snapshot(true, false);
 	AtCommit_ApplyLauncher();
 	pgstat_report_xact_timestamp(0);
 
@@ -2409,7 +2409,7 @@ PrepareTransaction(void)
 	AtEOXact_ComboCid();
 	AtEOXact_HashTables(true);
 	/* don't call AtEOXact_PgStat here; we fixed pgstat state above */
-	AtEOXact_Snapshot(true);
+	AtEOXact_Snapshot(true, true);
 	pgstat_report_xact_timestamp(0);
 
 	CurrentResourceOwner = NULL;
@@ -2640,7 +2640,7 @@ CleanupTransaction(void)
 	 * do abort cleanup processing
 	 */
 	AtCleanup_Portals();		/* now safe to release portal memory */
-	AtEOXact_Snapshot(false);	/* and release the transaction's snapshots */
+	AtEOXact_Snapshot(false, false); /* and release the transaction's snapshots */
 
 	CurrentResourceOwner = NULL;	/* and resource owner */
 	if (TopTransactionResourceOwner)
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index ecc32cb3fe..dff3d51ec3 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -1051,7 +1051,7 @@ AtSubAbort_Snapshot(int level)
  *		Snapshot manager's cleanup function for end of transaction
  */
 void
-AtEOXact_Snapshot(bool isCommit)
+AtEOXact_Snapshot(bool isCommit, bool isPrepare)
 {
 	/*
 	 * In transaction-snapshot mode we must release our privately-managed
@@ -1136,7 +1136,17 @@ AtEOXact_Snapshot(bool isCommit)
 
 	FirstSnapshotSet = false;
 
-	SnapshotResetXmin();
+	/*
+	 * During normal commit and abort processing, we call
+	 * ProcArrayEndTransaction() or ProcArrayClearTransaction() to
+	 * reset the PgXact->xmin. That call happens prior to the call to
+	 * AtEOXact_Snapshot(), so we need not touch xmin here at all,
+	 * accept when we are preparing a transaction.
+	 */
+	if (isPrepare)
+		SnapshotResetXmin();
+
+	Assert(isPrepare || MyPgXact->xmin == 0);
 }
 
 
diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h
index 86c48d9c8e..2f8d4fd4a0 100644
--- a/src/include/utils/snapmgr.h
+++ b/src/include/utils/snapmgr.h
@@ -85,7 +85,7 @@ extern void UnregisterSnapshotFromOwner(Snapshot snapshot, ResourceOwner owner);
 
 extern void AtSubCommit_Snapshot(int level);
 extern void AtSubAbort_Snapshot(int level);
-extern void AtEOXact_Snapshot(bool isCommit);
+extern void AtEOXact_Snapshot(bool isCommit, bool isPrepare);
 
 extern void ImportSnapshot(const char *idstr);
 extern bool XactHasExportedSnapshots(void);