diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2017-10-11 17:43:50 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2017-10-11 17:44:09 -0400 |
commit | 5fa6b0d102eb8ccd15c4963ee9841baec50df45e (patch) | |
tree | f35515f23f5802fbf3f8506778ceb9363f7e33a3 /src/backend/storage/large_object/inv_api.c | |
parent | f676616651c83b14e1d879fbfabdd3ab2dc70bbe (diff) | |
download | postgresql-5fa6b0d102eb8ccd15c4963ee9841baec50df45e.tar.gz postgresql-5fa6b0d102eb8ccd15c4963ee9841baec50df45e.zip |
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
Diffstat (limited to 'src/backend/storage/large_object/inv_api.c')
-rw-r--r-- | src/backend/storage/large_object/inv_api.c | 44 |
1 files changed, 13 insertions, 31 deletions
diff --git a/src/backend/storage/large_object/inv_api.c b/src/backend/storage/large_object/inv_api.c index d55d40e6f81..aa43b46c305 100644 --- a/src/backend/storage/large_object/inv_api.c +++ b/src/backend/storage/large_object/inv_api.c @@ -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; |