Tweak indexscan and seqscan code to arrange that steps from one page to

the next are handled by ReleaseAndReadBuffer rather than separate
ReleaseBuffer and ReadBuffer calls.  This cuts the number of acquisitions
of the BufMgrLock by a factor of 2 (possibly more, if an indexscan happens
to pull successive rows from the same heap page).  Unfortunately this
doesn't seem enough to get us out of the recently discussed context-switch
storm problem, but it's surely worth doing anyway.

3 files changed, 28 insertions, 6 deletions

diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c
index 347096a20eb..a68dbf1dca9 100644
--- 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.92 2004/02/28 19:46:05 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.93 2004/04/21 18:24:26 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -129,6 +129,15 @@ IndexNext(IndexScanState *node)
 	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
 	 * introduce new nodes for this case and handle IndexScan --> NewNode
@@ -139,7 +148,6 @@ IndexNext(IndexScanState *node)
 	{
 		List	   *qual;
 
-		ExecClearTuple(slot);
 		if (estate->es_evTupleNull[scanrelid - 1])
 			return slot;		/* return empty slot */
 
diff --git a/src/backend/executor/nodeSeqscan.c b/src/backend/executor/nodeSeqscan.c
index 97a765453a2..17461a827f6 100644
--- 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.47 2003/11/29 19:51:48 pgsql Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeSeqscan.c,v 1.48 2004/04/21 18:24:26 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -62,6 +62,15 @@ SeqNext(SeqScanState *node)
 	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
 	 * introduce new nodes for this case and handle SeqScan --> NewNode
@@ -70,7 +79,6 @@ SeqNext(SeqScanState *node)
 	if (estate->es_evTuple != NULL &&
 		estate->es_evTuple[scanrelid - 1] != NULL)
 	{
-		ExecClearTuple(slot);
 		if (estate->es_evTupleNull[scanrelid - 1])
 			return slot;		/* return empty slot */
 
diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c
index 2ee58655ed6..ee928deff5d 100644
--- 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.36 2003/11/29 19:51:48 pgsql Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeTidscan.c,v 1.37 2004/04/21 18:24:26 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -107,6 +107,13 @@ TidNext(TidScanState *node)
 	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
 	 * introduce new nodes for this case and handle TidScan --> NewNode
@@ -115,7 +122,6 @@ TidNext(TidScanState *node)
 	if (estate->es_evTuple != NULL &&
 		estate->es_evTuple[scanrelid - 1] != NULL)
 	{
-		ExecClearTuple(slot);
 		if (estate->es_evTupleNull[scanrelid - 1])
 			return slot;		/* return empty slot */