Remove explicit FreeExprContext calls during plan node shutdown. The

tglsfdc · tglsfdc · commit 9b5b9616f49e · 2005-04-23T21:32:34.000Z
ExprContexts will be freed anyway when FreeExecutorState() is reached,
and letting that routine do the work is more efficient because it will
automatically free the ExprContexts in reverse creation order.  The
existing coding was effectively freeing them in exactly the worst
possible order, resulting in O(N^2) behavior inside list_delete_ptr,
which becomes highly visible in cases with a few thousand plan nodes.

ExecFreeExprContext is now effectively a no-op and could be removed,
but I left it in place in case we ever want to put it back to use.
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.121 2005/04/14 20:03:24 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/execUtils.c,v 1.122 2005/04/23 21:32:34 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -620,27 +620,26 @@ ExecAssignProjectionInfo(PlanState *planstate)
 /* ----------------
  *		ExecFreeExprContext
  *
- * A plan node's ExprContext should be freed explicitly during ExecEndNode
- * because there may be shutdown callbacks to call.  (Other resources made
- * by the above routines, such as projection info, don't need to be freed
+ * A plan node's ExprContext should be freed explicitly during executor
+ * shutdown because there may be shutdown callbacks to call.  (Other resources
+ * made by the above routines, such as projection info, don't need to be freed
  * explicitly because they're just memory in the per-query memory context.)
+ *
+ * However ... there is no particular need to do it during ExecEndNode,
+ * because FreeExecutorState will free any remaining ExprContexts within
+ * the EState.  Letting FreeExecutorState do it allows the ExprContexts to
+ * be freed in reverse order of creation, rather than order of creation as
+ * will happen if we delete them here, which saves O(N^2) work in the list
+ * cleanup inside FreeExprContext.
  * ----------------
  */
 void
 ExecFreeExprContext(PlanState *planstate)
 {
-	ExprContext *econtext;
-
 	/*
-	 * get expression context.	if NULL then this node has none so we just
-	 * return.
+	 * Per above discussion, don't actually delete the ExprContext.
+	 * We do unlink it from the plan node, though.
 	 */
-	econtext = planstate->ps_ExprContext;
-	if (econtext == NULL)
-		return;
-
-	FreeExprContext(econtext);
-
 	planstate->ps_ExprContext = NULL;
 }
 
diff --git a/src/backend/executor/nodeBitmapIndexscan.c b/src/backend/executor/nodeBitmapIndexscan.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/nodeBitmapIndexscan.c,v 1.3 2005/04/22 21:58:31 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeBitmapIndexscan.c,v 1.4 2005/04/23 21:32:34 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -220,11 +220,13 @@ ExecEndBitmapIndexScan(BitmapIndexScanState *node)
 	relation = node->ss.ss_currentRelation;
 
 	/*
-	 * Free the exprcontext(s)
+	 * Free the exprcontext(s) ... now dead code, see ExecFreeExprContext
 	 */
+#ifdef NOT_USED
 	ExecFreeExprContext(&node->ss.ps);
 	if (node->biss_RuntimeContext)
 		FreeExprContext(node->biss_RuntimeContext);
+#endif
 
 	/*
 	 * close the index relation
diff --git 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.100 2005/03/16 21:38:07 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeIndexscan.c,v 1.101 2005/04/23 21:32:34 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -534,11 +534,13 @@ ExecEndIndexScan(IndexScanState *node)
 	relation = node->ss.ss_currentRelation;
 
 	/*
-	 * Free the exprcontext(s)
+	 * Free the exprcontext(s) ... now dead code, see ExecFreeExprContext
 	 */
+#ifdef NOT_USED
 	ExecFreeExprContext(&node->ss.ps);
 	if (node->iss_RuntimeContext)
 		FreeExprContext(node->iss_RuntimeContext);
+#endif
 
 	/*
 	 * clear out tuple table slots
