Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit5fa6b0d

Browse files
committed
Remove unnecessary PG_TRY overhead for CurrentResourceOwner changes.
resowner/README contained advice to use a PG_TRY block to restore theold CurrentResourceOwner value anywhere that that variable is transientlychanged. That advice was only inconsistently followed, however, andon reflection it seems like unnecessary overhead. We don't botherwith such a convention for transient CurrentMemoryContext changes,on the grounds that any (sub)transaction abort will start out byresetting CurrentMemoryContext to what it wants. But the same istrue of CurrentResourceOwner, so there seems no need to treat itdifferently.Hence, remove PG_TRY blocks that exist only to restore CurrentResourceOwnerbefore re-throwing the error. There are a couple of places that restoreit along with some other actions, and I left those alone; the restore isprobably unnecessary but no noticeable gain will result from removing it.Discussion:https://postgr.es/m/5236.1507583529@sss.pgh.pa.us
1 parentf676616 commit5fa6b0d

File tree

7 files changed

+42
-116
lines changed

7 files changed

+42
-116
lines changed

‎src/backend/access/transam/xact.c

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -575,18 +575,10 @@ AssignTransactionId(TransactionState s)
575575
* ResourceOwner.
576576
*/
577577
currentOwner=CurrentResourceOwner;
578-
PG_TRY();
579-
{
580-
CurrentResourceOwner=s->curTransactionOwner;
581-
XactLockTableInsert(s->transactionId);
582-
}
583-
PG_CATCH();
584-
{
585-
/* Ensure CurrentResourceOwner is restored on error */
586-
CurrentResourceOwner=currentOwner;
587-
PG_RE_THROW();
588-
}
589-
PG_END_TRY();
578+
CurrentResourceOwner=s->curTransactionOwner;
579+
580+
XactLockTableInsert(s->transactionId);
581+
590582
CurrentResourceOwner=currentOwner;
591583

592584
/*

‎src/backend/commands/portalcmds.c

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -294,21 +294,13 @@ PortalCleanup(Portal portal)
294294

295295
/* We must make the portal's resource owner current */
296296
saveResourceOwner=CurrentResourceOwner;
297-
PG_TRY();
298-
{
299-
if (portal->resowner)
300-
CurrentResourceOwner=portal->resowner;
301-
ExecutorFinish(queryDesc);
302-
ExecutorEnd(queryDesc);
303-
FreeQueryDesc(queryDesc);
304-
}
305-
PG_CATCH();
306-
{
307-
/* Ensure CurrentResourceOwner is restored on error */
308-
CurrentResourceOwner=saveResourceOwner;
309-
PG_RE_THROW();
310-
}
311-
PG_END_TRY();
297+
if (portal->resowner)
298+
CurrentResourceOwner=portal->resowner;
299+
300+
ExecutorFinish(queryDesc);
301+
ExecutorEnd(queryDesc);
302+
FreeQueryDesc(queryDesc);
303+
312304
CurrentResourceOwner=saveResourceOwner;
313305
}
314306
}

‎src/backend/commands/sequence.c

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,18 +1055,10 @@ lock_and_open_sequence(SeqTable seq)
10551055
ResourceOwnercurrentOwner;
10561056

10571057
currentOwner=CurrentResourceOwner;
1058-
PG_TRY();
1059-
{
1060-
CurrentResourceOwner=TopTransactionResourceOwner;
1061-
LockRelationOid(seq->relid,RowExclusiveLock);
1062-
}
1063-
PG_CATCH();
1064-
{
1065-
/* Ensure CurrentResourceOwner is restored on error */
1066-
CurrentResourceOwner=currentOwner;
1067-
PG_RE_THROW();
1068-
}
1069-
PG_END_TRY();
1058+
CurrentResourceOwner=TopTransactionResourceOwner;
1059+
1060+
LockRelationOid(seq->relid,RowExclusiveLock);
1061+
10701062
CurrentResourceOwner=currentOwner;
10711063

10721064
/* Flag that we have a lock in the current xact */

‎src/backend/commands/trigger.c

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3639,17 +3639,10 @@ GetCurrentFDWTuplestore(void)
36393639
*/
36403640
oldcxt=MemoryContextSwitchTo(CurTransactionContext);
36413641
saveResourceOwner=CurrentResourceOwner;
3642-
PG_TRY();
3643-
{
3644-
CurrentResourceOwner=CurTransactionResourceOwner;
3645-
ret=tuplestore_begin_heap(false, false,work_mem);
3646-
}
3647-
PG_CATCH();
3648-
{
3649-
CurrentResourceOwner=saveResourceOwner;
3650-
PG_RE_THROW();
3651-
}
3652-
PG_END_TRY();
3642+
CurrentResourceOwner=CurTransactionResourceOwner;
3643+
3644+
ret=tuplestore_begin_heap(false, false,work_mem);
3645+
36533646
CurrentResourceOwner=saveResourceOwner;
36543647
MemoryContextSwitchTo(oldcxt);
36553648

@@ -4468,20 +4461,13 @@ MakeTransitionCaptureState(TriggerDesc *trigdesc, Oid relid, CmdType cmdType)
44684461
/* Now create required tuplestore(s), if we don't have them already. */
44694462
oldcxt=MemoryContextSwitchTo(CurTransactionContext);
44704463
saveResourceOwner=CurrentResourceOwner;
4471-
PG_TRY();
4472-
{
4473-
CurrentResourceOwner=CurTransactionResourceOwner;
4474-
if (need_old&&table->old_tuplestore==NULL)
4475-
table->old_tuplestore=tuplestore_begin_heap(false, false,work_mem);
4476-
if (need_new&&table->new_tuplestore==NULL)
4477-
table->new_tuplestore=tuplestore_begin_heap(false, false,work_mem);
4478-
}
4479-
PG_CATCH();
4480-
{
4481-
CurrentResourceOwner=saveResourceOwner;
4482-
PG_RE_THROW();
4483-
}
4484-
PG_END_TRY();
4464+
CurrentResourceOwner=CurTransactionResourceOwner;
4465+
4466+
if (need_old&&table->old_tuplestore==NULL)
4467+
table->old_tuplestore=tuplestore_begin_heap(false, false,work_mem);
4468+
if (need_new&&table->new_tuplestore==NULL)
4469+
table->new_tuplestore=tuplestore_begin_heap(false, false,work_mem);
4470+
44854471
CurrentResourceOwner=saveResourceOwner;
44864472
MemoryContextSwitchTo(oldcxt);
44874473

‎src/backend/storage/large_object/inv_api.c

Lines changed: 13 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -75,23 +75,14 @@ open_lo_relation(void)
7575

7676
/* Arrange for the top xact to own these relation references */
7777
currentOwner=CurrentResourceOwner;
78-
PG_TRY();
79-
{
80-
CurrentResourceOwner=TopTransactionResourceOwner;
78+
CurrentResourceOwner=TopTransactionResourceOwner;
79+
80+
/* Use RowExclusiveLock since we might either read or write */
81+
if (lo_heap_r==NULL)
82+
lo_heap_r=heap_open(LargeObjectRelationId,RowExclusiveLock);
83+
if (lo_index_r==NULL)
84+
lo_index_r=index_open(LargeObjectLOidPNIndexId,RowExclusiveLock);
8185

82-
/* Use RowExclusiveLock since we might either read or write */
83-
if (lo_heap_r==NULL)
84-
lo_heap_r=heap_open(LargeObjectRelationId,RowExclusiveLock);
85-
if (lo_index_r==NULL)
86-
lo_index_r=index_open(LargeObjectLOidPNIndexId,RowExclusiveLock);
87-
}
88-
PG_CATCH();
89-
{
90-
/* Ensure CurrentResourceOwner is restored on error */
91-
CurrentResourceOwner=currentOwner;
92-
PG_RE_THROW();
93-
}
94-
PG_END_TRY();
9586
CurrentResourceOwner=currentOwner;
9687
}
9788

@@ -112,22 +103,13 @@ close_lo_relation(bool isCommit)
112103
ResourceOwnercurrentOwner;
113104

114105
currentOwner=CurrentResourceOwner;
115-
PG_TRY();
116-
{
117-
CurrentResourceOwner=TopTransactionResourceOwner;
106+
CurrentResourceOwner=TopTransactionResourceOwner;
107+
108+
if (lo_index_r)
109+
index_close(lo_index_r,NoLock);
110+
if (lo_heap_r)
111+
heap_close(lo_heap_r,NoLock);
118112

119-
if (lo_index_r)
120-
index_close(lo_index_r,NoLock);
121-
if (lo_heap_r)
122-
heap_close(lo_heap_r,NoLock);
123-
}
124-
PG_CATCH();
125-
{
126-
/* Ensure CurrentResourceOwner is restored on error */
127-
CurrentResourceOwner=currentOwner;
128-
PG_RE_THROW();
129-
}
130-
PG_END_TRY();
131113
CurrentResourceOwner=currentOwner;
132114
}
133115
lo_heap_r=NULL;

‎src/backend/utils/resowner/README

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,3 @@ CurrentResourceOwner must point to the same resource owner that was current
8080
when the buffer, lock, or cache reference was acquired. It would be possible
8181
to relax this restriction given additional bookkeeping effort, but at present
8282
there seems no need.
83-
84-
Code that transiently changes CurrentResourceOwner should generally use a
85-
PG_TRY construct to ensure that the previous value of CurrentResourceOwner
86-
is restored if control is lost during an error exit.

‎src/backend/utils/resowner/resowner.c

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -473,21 +473,8 @@ ResourceOwnerRelease(ResourceOwner owner,
473473
boolisCommit,
474474
boolisTopLevel)
475475
{
476-
/* Rather than PG_TRY at every level of recursion, set it up once */
477-
ResourceOwnersave;
478-
479-
save=CurrentResourceOwner;
480-
PG_TRY();
481-
{
482-
ResourceOwnerReleaseInternal(owner,phase,isCommit,isTopLevel);
483-
}
484-
PG_CATCH();
485-
{
486-
CurrentResourceOwner=save;
487-
PG_RE_THROW();
488-
}
489-
PG_END_TRY();
490-
CurrentResourceOwner=save;
476+
/* There's not currently any setup needed before recursing */
477+
ResourceOwnerReleaseInternal(owner,phase,isCommit,isTopLevel);
491478
}
492479

493480
staticvoid
@@ -507,8 +494,7 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
507494

508495
/*
509496
* Make CurrentResourceOwner point to me, so that ReleaseBuffer etc don't
510-
* get confused. We needn't PG_TRY here because the outermost level will
511-
* fix it on error abort.
497+
* get confused.
512498
*/
513499
save=CurrentResourceOwner;
514500
CurrentResourceOwner=owner;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp