Improve ExecStoreTuple to be smarter about replacing the contents of

a TupleTableSlot: instead of calling ExecClearTuple, inline the needed
operations, so that we can avoid redundant steps.  In particular, when
the old and new tuples are both on the same disk page, avoid releasing
and re-acquiring the buffer pin --- this saves work in both the bufmgr
and ResourceOwner modules.  To make this improvement actually useful,
partially revert a change I made on 2004-04-21 that caused SeqNext
et al to call ExecClearTuple before ExecStoreTuple.  The motivation
for that, to avoid grabbing the BufMgrLock separately for releasing
the old buffer and grabbing the new one, no longer applies.  My
profiling says that this saves about 5% of the CPU time for an
all-in-memory seqscan.

diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c
index 7a4f11f5d9f9362e644c021c38e288401f00a1ee..09c7dd55f5013f407d9aca925f9d09750a152bcc 100644 (file)
--- a/src/backend/executor/execTuples.c
+++ b/src/backend/executor/execTuples.c
@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/executor/execTuples.c,v 1.89 2005/11/22 18:17:10 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/executor/execTuples.c,v 1.90 2005/11/25 04:24:48 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -402,28 +402,38 @@ ExecStoreTuple(HeapTuple tuple,
    Assert(BufferIsValid(buffer) ? (!shouldFree) : true);
 
    /*
-    * clear out any old contents of the slot
+    * Free any old physical tuple belonging to the slot.
     */
-   if (!slot->tts_isempty)
-       ExecClearTuple(slot);
+   if (slot->tts_shouldFree)
+       heap_freetuple(slot->tts_tuple);
 
    /*
-    * store the new tuple into the specified slot.
+    * Store the new tuple into the specified slot.
     */
    slot->tts_isempty = false;
    slot->tts_shouldFree = shouldFree;
    slot->tts_tuple = tuple;
 
+   /* Mark extracted state invalid */
+   slot->tts_nvalid = 0;
+
    /*
     * If tuple is on a disk page, keep the page pinned as long as we hold a
     * pointer into it.  We assume the caller already has such a pin.
+    *
+    * This is coded to optimize the case where the slot previously held a
+    * tuple on the same disk page: in that case releasing and re-acquiring
+    * the pin is a waste of cycles.  This is a common situation during
+    * seqscans, so it's worth troubling over.
     */
-   slot->tts_buffer = buffer;
-   if (BufferIsValid(buffer))
-       IncrBufferRefCount(buffer);
-
-   /* Mark extracted state invalid */
-   slot->tts_nvalid = 0;
+   if (slot->tts_buffer != buffer)
+   {
+       if (BufferIsValid(slot->tts_buffer))
+           ReleaseBuffer(slot->tts_buffer);
+       slot->tts_buffer = buffer;
+       if (BufferIsValid(buffer))
+           IncrBufferRefCount(buffer);
+   }
 
    return slot;
 }

diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 5d92c19ea5e915eea53952d88c26e7ae1394cbcb..3d4f7d38392a5525a9b790ff2ff3a8733e889d1b 100644 (file)
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -21,7 +21,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/executor/nodeBitmapHeapscan.c,v 1.4 2005/10/15 02:49:17 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/executor/nodeBitmapHeapscan.c,v 1.5 2005/11/25 04:24:48 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -75,15 +75,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
    tbm = node->tbm;
    tbmres = node->tbmres;
 
-   /*
-    * Clear any reference to the previously returned tuple.  The idea here is
-    * to not have the tuple slot be the last holder of a pin on that tuple's
-    * buffer; if it is, we'll need a separate visit to the bufmgr to release
-    * the buffer.  By clearing here, we get to have the release done by
-    * ReleaseAndReadBuffer, below.
-    */
-   ExecClearTuple(slot);
-
    /*
     * Check if we are evaluating PlanQual for tuple of this relation.
     * Additional checking is not good, but no other way for now. We could
@@ -94,7 +85,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
        estate->es_evTuple[scanrelid - 1] != NULL)
    {
        if (estate->es_evTupleNull[scanrelid - 1])
-           return slot;        /* return empty slot */
+           return ExecClearTuple(slot);
 
        ExecStoreTuple(estate->es_evTuple[scanrelid - 1],
                       slot, InvalidBuffer, false);

diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c
index 6e639502c1d4b7a96ffe283b67ff2c6f083f5902..4f6fadfde490fce28df8c8f45964e94f7f470cfe 100644 (file)
--- a/src/backend/executor/nodeIndexscan.c
+++ b/src/backend/executor/nodeIndexscan.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.105 2005/11/22 18:17:10 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.106 2005/11/25 04:24:48 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -74,15 +74,6 @@ IndexNext(IndexScanState *node)
    slot = node->ss.ss_ScanTupleSlot;
    scanrelid = ((IndexScan *) node->ss.ps.plan)->scan.scanrelid;
 
-   /*
-    * Clear any reference to the previously returned tuple.  The idea here is
-    * to not have the tuple slot be the last holder of a pin on that tuple's
-    * buffer; if it is, we'll need a separate visit to the bufmgr to release
-    * the buffer.  By clearing here, we get to have the release done by
-    * ReleaseAndReadBuffer inside index_getnext.
-    */
-   ExecClearTuple(slot);
-
    /*
     * Check if we are evaluating PlanQual for tuple of this relation.
     * Additional checking is not good, but no other way for now. We could
@@ -93,7 +84,7 @@ IndexNext(IndexScanState *node)
        estate->es_evTuple[scanrelid - 1] != NULL)
    {
        if (estate->es_evTupleNull[scanrelid - 1])
-           return slot;        /* return empty slot */
+           return ExecClearTuple(slot);
 
        ExecStoreTuple(estate->es_evTuple[scanrelid - 1],
                       slot, InvalidBuffer, false);

diff --git a/src/backend/executor/nodeSeqscan.c b/src/backend/executor/nodeSeqscan.c
index 91e0c81e036890a5f22fb1ef04e8cb2be102a42c..391fdf7e9152b928f4475faef57fee00c6873858 100644 (file)
--- a/src/backend/executor/nodeSeqscan.c
+++ b/src/backend/executor/nodeSeqscan.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/executor/nodeSeqscan.c,v 1.54 2005/10/15 02:49:17 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/executor/nodeSeqscan.c,v 1.55 2005/11/25 04:24:48 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -61,15 +61,6 @@ SeqNext(SeqScanState *node)
    direction = estate->es_direction;
    slot = node->ss_ScanTupleSlot;
 
-   /*
-    * Clear any reference to the previously returned tuple.  The idea here is
-    * to not have the tuple slot be the last holder of a pin on that tuple's
-    * buffer; if it is, we'll need a separate visit to the bufmgr to release
-    * the buffer.  By clearing here, we get to have the release done by
-    * ReleaseAndReadBuffer inside heap_getnext.
-    */
-   ExecClearTuple(slot);
-
    /*
     * Check if we are evaluating PlanQual for tuple of this relation.
     * Additional checking is not good, but no other way for now. We could
@@ -80,7 +71,7 @@ SeqNext(SeqScanState *node)
        estate->es_evTuple[scanrelid - 1] != NULL)
    {
        if (estate->es_evTupleNull[scanrelid - 1])
-           return slot;        /* return empty slot */
+           return ExecClearTuple(slot);
 
        ExecStoreTuple(estate->es_evTuple[scanrelid - 1],
                       slot, InvalidBuffer, false);
@@ -93,7 +84,7 @@ SeqNext(SeqScanState *node)
 
        /* Flag for the next call that no more tuples */
        estate->es_evTupleNull[scanrelid - 1] = true;
-       return (slot);
+       return slot;
    }
 
    /*
@@ -115,6 +106,8 @@ SeqNext(SeqScanState *node)
                       scandesc->rs_cbuf,       /* buffer associated with this
                                                 * tuple */
                       false);  /* don't pfree this pointer */
+   else
+       ExecClearTuple(slot);
 
    return slot;
 }

diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c
index c8708f583110e2c492be92e0db94bb39754216d5..4b0775719e5c0b324fa7aaaf7fdf2d885ee239a7 100644 (file)
--- a/src/backend/executor/nodeTidscan.c
+++ b/src/backend/executor/nodeTidscan.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/executor/nodeTidscan.c,v 1.43 2005/10/15 02:49:17 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/executor/nodeTidscan.c,v 1.44 2005/11/25 04:24:48 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -106,13 +106,6 @@ TidNext(TidScanState *node)
    slot = node->ss.ss_ScanTupleSlot;
    scanrelid = ((TidScan *) node->ss.ps.plan)->scan.scanrelid;
 
-   /*
-    * Clear any reference to the previously returned tuple.  This doesn't
-    * offer any great performance benefit, but it keeps this code in sync
-    * with SeqNext and IndexNext.
-    */
-   ExecClearTuple(slot);
-
    /*
     * Check if we are evaluating PlanQual for tuple of this relation.
     * Additional checking is not good, but no other way for now. We could
@@ -123,7 +116,7 @@ TidNext(TidScanState *node)
        estate->es_evTuple[scanrelid - 1] != NULL)
    {
        if (estate->es_evTupleNull[scanrelid - 1])
-           return slot;        /* return empty slot */
+           return ExecClearTuple(slot);
 
        /*
         * XXX shouldn't we check here to make sure tuple matches TID list? In
@@ -135,7 +128,7 @@ TidNext(TidScanState *node)
 
        /* Flag for the next call that no more tuples */
        estate->es_evTupleNull[scanrelid - 1] = true;
-       return (slot);
+       return slot;
    }
 
    /*