From 83cd2d8b0f1af64846337dd9b34d8e362309bc92 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 26 Oct 2004 16:05:03 +0000 Subject: [PATCH] Make heap_fetch API more consistent by having the buffer remain pinned in all cases when keep_buf = true. This allows ANALYZE's inner loop to use heap_release_fetch, which saves multiple buffer lookups for the same page and avoids overestimation of cost by the vacuum cost mechanism. --- src/backend/access/heap/heapam.c | 41 +++++++++++++++------------ src/backend/access/nbtree/nbtinsert.c | 4 +-- src/backend/commands/analyze.c | 14 ++++----- 3 files changed, 31 insertions(+), 28 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index af5325ea63..c2d4395807 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.179 2004/10/15 22:39:42 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.180 2004/10/26 16:05:02 tgl Exp $ * * * INTERFACE ROUTINES @@ -878,14 +878,16 @@ heap_getnext(HeapScanDesc scan, ScanDirection direction) * is set to the buffer holding the tuple and TRUE is returned. The caller * must unpin the buffer when done with the tuple. * - * If the tuple is not found, then tuple->t_data is set to NULL, *userbuf - * is set to InvalidBuffer, and FALSE is returned. + * If the tuple is not found (ie, item number references a deleted slot), + * then tuple->t_data is set to NULL and FALSE is returned. * - * If the tuple is found but fails the time qual check, then FALSE will be - * returned. When the caller specifies keep_buf = true, we retain the pin - * on the buffer and return it in *userbuf (so the caller can still access - * the tuple); when keep_buf = false, the pin is released and *userbuf is set - * to InvalidBuffer. + * If the tuple is found but fails the time qual check, then FALSE is returned + * but tuple->t_data is left pointing to the tuple. + * + * keep_buf determines what is done with the buffer in the FALSE-result cases. + * When the caller specifies keep_buf = true, we retain the pin on the buffer + * and return it in *userbuf (so the caller must eventually unpin it); when + * keep_buf = false, the pin is released and *userbuf is set to InvalidBuffer. * * It is somewhat inconsistent that we ereport() on invalid block number but * return false on invalid item number. This is historical. The only @@ -914,6 +916,8 @@ heap_fetch(Relation relation, * InvalidBuffer on entry, that buffer will be released before reading * the new page. This saves a separate ReleaseBuffer step and hence * one entry into the bufmgr when looping through multiple fetches. + * Also, if *userbuf is the same buffer that holds the target tuple, + * we avoid bufmgr manipulation altogether. */ bool heap_release_fetch(Relation relation, @@ -962,8 +966,13 @@ heap_release_fetch(Relation relation, if (!ItemIdIsUsed(lp)) { LockBuffer(buffer, BUFFER_LOCK_UNLOCK); - ReleaseBuffer(buffer); - *userbuf = InvalidBuffer; + if (keep_buf) + *userbuf = buffer; + else + { + ReleaseBuffer(buffer); + *userbuf = InvalidBuffer; + } tuple->t_datamcxt = NULL; tuple->t_data = NULL; return false; @@ -1007,17 +1016,13 @@ heap_release_fetch(Relation relation, /* Tuple failed time qual, but maybe caller wants to see it anyway. */ if (keep_buf) - { *userbuf = buffer; - - return false; + else + { + ReleaseBuffer(buffer); + *userbuf = InvalidBuffer; } - /* Okay to release pin on buffer. */ - ReleaseBuffer(buffer); - - *userbuf = InvalidBuffer; - return false; } diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c index 32615a62c2..9f22cb2040 100644 --- a/src/backend/access/nbtree/nbtinsert.c +++ b/src/backend/access/nbtree/nbtinsert.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/nbtree/nbtinsert.c,v 1.117 2004/10/15 22:39:49 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/access/nbtree/nbtinsert.c,v 1.118 2004/10/26 16:05:02 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -269,8 +269,8 @@ _bt_check_unique(Relation rel, BTItem btitem, Relation heapRel, SetBufferCommitInfoNeedsSave(buf); } LockBuffer(hbuffer, BUFFER_LOCK_UNLOCK); - ReleaseBuffer(hbuffer); } + ReleaseBuffer(hbuffer); } } diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 486e86d677..872edc4298 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.77 2004/09/30 23:21:19 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.78 2004/10/26 16:05:03 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -822,11 +822,12 @@ acquire_sample_rows(Relation onerel, HeapTuple *rows, int targrows, for (targoffset = FirstOffsetNumber; targoffset <= maxoffset; targoffset++) { HeapTupleData targtuple; - Buffer tupbuffer; ItemPointerSet(&targtuple.t_self, targblock, targoffset); - if (heap_fetch(onerel, SnapshotNow, &targtuple, &tupbuffer, - false, NULL)) + /* We use heap_release_fetch to avoid useless bufmgr traffic */ + if (heap_release_fetch(onerel, SnapshotNow, + &targtuple, &targbuffer, + true, NULL)) { /* * The first targrows live rows are simply copied into the @@ -869,9 +870,6 @@ acquire_sample_rows(Relation onerel, HeapTuple *rows, int targrows, rowstoskip -= 1; } - /* must release the extra pin acquired by heap_fetch */ - ReleaseBuffer(tupbuffer); - liverows += 1; } else @@ -886,7 +884,7 @@ acquire_sample_rows(Relation onerel, HeapTuple *rows, int targrows, } } - /* Now release the initial pin on the page */ + /* Now release the pin on the page */ ReleaseBuffer(targbuffer); }