From 5fa6b0d102eb8ccd15c4963ee9841baec50df45e Mon Sep 17 00:00:00 2001
From: Tom Lane
Date: Wed, 11 Oct 2017 17:43:50 -0400
Subject: Remove unnecessary PG_TRY overhead for CurrentResourceOwner changes.

resowner/README contained advice to use a PG_TRY block to restore the
old CurrentResourceOwner value anywhere that that variable is transiently
changed.  That advice was only inconsistently followed, however, and
on reflection it seems like unnecessary overhead.  We don't bother
with such a convention for transient CurrentMemoryContext changes,
on the grounds that any (sub)transaction abort will start out by
resetting CurrentMemoryContext to what it wants.  But the same is
true of CurrentResourceOwner, so there seems no need to treat it
differently.

Hence, remove PG_TRY blocks that exist only to restore CurrentResourceOwner
before re-throwing the error.  There are a couple of places that restore
it along with some other actions, and I left those alone; the restore is
probably unnecessary but no noticeable gain will result from removing it.

Discussion: https://postgr.es/m/5236.1507583529@sss.pgh.pa.us
---
 src/backend/utils/resowner/README     |  4 ----
 src/backend/utils/resowner/resowner.c | 20 +++-----------------
 2 files changed, 3 insertions(+), 21 deletions(-)

(limited to 'src/backend/utils')

diff --git a/src/backend/utils/resowner/README b/src/backend/utils/resowner/README
index e7b9ddfa59a..2998f6bb362 100644
--- a/src/backend/utils/resowner/README
+++ b/src/backend/utils/resowner/README
@@ -80,7 +80,3 @@ CurrentResourceOwner must point to the same resource owner that was current
 when the buffer, lock, or cache reference was acquired.  It would be possible
 to relax this restriction given additional bookkeeping effort, but at present
 there seems no need.
-
-Code that transiently changes CurrentResourceOwner should generally use a
-PG_TRY construct to ensure that the previous value of CurrentResourceOwner
-is restored if control is lost during an error exit.
diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index bd19fad77e9..4c35ccf65eb 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -473,21 +473,8 @@ ResourceOwnerRelease(ResourceOwner owner,
 					 bool isCommit,
 					 bool isTopLevel)
 {
-	/* Rather than PG_TRY at every level of recursion, set it up once */
-	ResourceOwner save;
-
-	save = CurrentResourceOwner;
-	PG_TRY();
-	{
-		ResourceOwnerReleaseInternal(owner, phase, isCommit, isTopLevel);
-	}
-	PG_CATCH();
-	{
-		CurrentResourceOwner = save;
-		PG_RE_THROW();
-	}
-	PG_END_TRY();
-	CurrentResourceOwner = save;
+	/* There's not currently any setup needed before recursing */
+	ResourceOwnerReleaseInternal(owner, phase, isCommit, isTopLevel);
 }
 
 static void
@@ -507,8 +494,7 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 
 	/*
 	 * Make CurrentResourceOwner point to me, so that ReleaseBuffer etc don't
-	 * get confused.  We needn't PG_TRY here because the outermost level will
-	 * fix it on error abort.
+	 * get confused.
 	 */
 	save = CurrentResourceOwner;
 	CurrentResourceOwner = owner;
-- 
cgit v1.2.3