mirror/postgresql
2
0
Fork 0
mirror of https://git.postgresql.org/git/postgresql.git synced 2025-01-30 19:00:29 +08:00
Code Issues Packages Projects Releases Wiki Activity

Remove unnecessary PG_TRY overhead for CurrentResourceOwner changes.

Browse Source 
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
This commit is contained in:
Tom Lane 2017-10-11 17:43:50 -04:00
parent f676616651
commit 5fa6b0d102
7 changed files with 42 additions and 116 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/backend/access/transam/xact.c
View File

@ -575,18 +575,10 @@ AssignTransactionId(TransactionState s)
* ResourceOwner.
*/
currentOwner = CurrentResourceOwner;
PG_TRY();
{
CurrentResourceOwner = s->curTransactionOwner;
XactLockTableInsert(s->transactionId);
}
PG_CATCH();
{
/* Ensure CurrentResourceOwner is restored on error */
CurrentResourceOwner = currentOwner;
PG_RE_THROW();
}
PG_END_TRY();
CurrentResourceOwner = s->curTransactionOwner;
XactLockTableInsert(s->transactionId);
CurrentResourceOwner = currentOwner;
/*

22
src/backend/commands/portalcmds.c
View File

@ -294,21 +294,13 @@ PortalCleanup(Portal portal)
/* We must make the portal's resource owner current */
saveResourceOwner = CurrentResourceOwner;
PG_TRY();
{
if (portal->resowner)
CurrentResourceOwner = portal->resowner;
ExecutorFinish(queryDesc);
ExecutorEnd(queryDesc);
FreeQueryDesc(queryDesc);
}
PG_CATCH();
{
/* Ensure CurrentResourceOwner is restored on error */
CurrentResourceOwner = saveResourceOwner;
PG_RE_THROW();
}
PG_END_TRY();
if (portal->resowner)
CurrentResourceOwner = portal->resowner;
ExecutorFinish(queryDesc);
ExecutorEnd(queryDesc);
FreeQueryDesc(queryDesc);
CurrentResourceOwner = saveResourceOwner;
}
}

16
src/backend/commands/sequence.c
View File

@ -1055,18 +1055,10 @@ lock_and_open_sequence(SeqTable seq)
ResourceOwner currentOwner;
currentOwner = CurrentResourceOwner;
PG_TRY();
{
CurrentResourceOwner = TopTransactionResourceOwner;
LockRelationOid(seq->relid, RowExclusiveLock);
}
PG_CATCH();
{
/* Ensure CurrentResourceOwner is restored on error */
CurrentResourceOwner = currentOwner;
PG_RE_THROW();
}
PG_END_TRY();
CurrentResourceOwner = TopTransactionResourceOwner;
LockRelationOid(seq->relid, RowExclusiveLock);
CurrentResourceOwner = currentOwner;
/* Flag that we have a lock in the current xact */

36
src/backend/commands/trigger.c
View File

@ -3639,17 +3639,10 @@ GetCurrentFDWTuplestore(void)
*/
oldcxt = MemoryContextSwitchTo(CurTransactionContext);
saveResourceOwner = CurrentResourceOwner;
PG_TRY();
{
CurrentResourceOwner = CurTransactionResourceOwner;
ret = tuplestore_begin_heap(false, false, work_mem);
}
PG_CATCH();
{
CurrentResourceOwner = saveResourceOwner;
PG_RE_THROW();
}
PG_END_TRY();
CurrentResourceOwner = CurTransactionResourceOwner;
ret = tuplestore_begin_heap(false, false, work_mem);
CurrentResourceOwner = saveResourceOwner;
MemoryContextSwitchTo(oldcxt);
@ -4468,20 +4461,13 @@ MakeTransitionCaptureState(TriggerDesc *trigdesc, Oid relid, CmdType cmdType)
/* Now create required tuplestore(s), if we don't have them already. */
oldcxt = MemoryContextSwitchTo(CurTransactionContext);
saveResourceOwner = CurrentResourceOwner;
PG_TRY();
{
CurrentResourceOwner = CurTransactionResourceOwner;
if (need_old && table->old_tuplestore == NULL)
table->old_tuplestore = tuplestore_begin_heap(false, false, work_mem);
if (need_new && table->new_tuplestore == NULL)
table->new_tuplestore = tuplestore_begin_heap(false, false, work_mem);
}
PG_CATCH();
{
CurrentResourceOwner = saveResourceOwner;
PG_RE_THROW();
}
PG_END_TRY();
CurrentResourceOwner = CurTransactionResourceOwner;
if (need_old && table->old_tuplestore == NULL)
table->old_tuplestore = tuplestore_begin_heap(false, false, work_mem);
if (need_new && table->new_tuplestore == NULL)
table->new_tuplestore = tuplestore_begin_heap(false, false, work_mem);
CurrentResourceOwner = saveResourceOwner;
MemoryContextSwitchTo(oldcxt);

44
src/backend/storage/large_object/inv_api.c
View File

@ -75,23 +75,14 @@ open_lo_relation(void)
/* Arrange for the top xact to own these relation references */
currentOwner = CurrentResourceOwner;
PG_TRY();
{
CurrentResourceOwner = TopTransactionResourceOwner;
CurrentResourceOwner = TopTransactionResourceOwner;
/* Use RowExclusiveLock since we might either read or write */
if (lo_heap_r == NULL)
lo_heap_r = heap_open(LargeObjectRelationId, RowExclusiveLock);
if (lo_index_r == NULL)
lo_index_r = index_open(LargeObjectLOidPNIndexId, RowExclusiveLock);
/* Use RowExclusiveLock since we might either read or write */
if (lo_heap_r == NULL)
lo_heap_r = heap_open(LargeObjectRelationId, RowExclusiveLock);
if (lo_index_r == NULL)
lo_index_r = index_open(LargeObjectLOidPNIndexId, RowExclusiveLock);
}
PG_CATCH();
{
/* Ensure CurrentResourceOwner is restored on error */
CurrentResourceOwner = currentOwner;
PG_RE_THROW();
}
PG_END_TRY();
CurrentResourceOwner = currentOwner;
}
@ -112,22 +103,13 @@ close_lo_relation(bool isCommit)
ResourceOwner currentOwner;
currentOwner = CurrentResourceOwner;
PG_TRY();
{
CurrentResourceOwner = TopTransactionResourceOwner;
CurrentResourceOwner = TopTransactionResourceOwner;
if (lo_index_r)
index_close(lo_index_r, NoLock);
if (lo_heap_r)
heap_close(lo_heap_r, NoLock);
if (lo_index_r)
index_close(lo_index_r, NoLock);
if (lo_heap_r)
heap_close(lo_heap_r, NoLock);
}
PG_CATCH();
{
/* Ensure CurrentResourceOwner is restored on error */
CurrentResourceOwner = currentOwner;
PG_RE_THROW();
}
PG_END_TRY();
CurrentResourceOwner = currentOwner;
}
lo_heap_r = NULL;

4
src/backend/utils/resowner/README
View File

@ -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.

20
src/backend/utils/resowner/resowner.c
View File

@ -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;