Speedup WindowAgg code by moving uncommon code out-of-line

The code to calculate the frame offsets is only performed once per scan.
Moving this code out of line gives a small (around 4-5%) speedup when testing
with some CPUs.  Other tested CPUs are indifferent to the change.

Reviewed-by: Ashutosh Bapat <[email protected]>
Reviewed-by: Tatsuo Ishii <[email protected]>
Discussion: https://postgr.es/m/CAApHDvqPgFtwme2Zyf75BpMLwYr2mnUstDyPiP%3DEpudYuQTPPQ%40mail.gmail.com

diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index 3221fa1522a3a2ae46e709193e13cbc3349dfcf0..88a85f556b62ffe9fa6a1aa963daf4dd9a8e86e8 100644 (file)
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -2032,6 +2032,82 @@ update_grouptailpos(WindowAggState *winstate)
        MemoryContextSwitchTo(oldcontext);
 }
 
+/*
+ * calculate_frame_offsets
+ *             Determine the startOffsetValue and endOffsetValue values for the
+ *             WindowAgg's frame options.
+ */
+static pg_noinline void
+calculate_frame_offsets(PlanState *pstate)
+{
+       WindowAggState *winstate = castNode(WindowAggState, pstate);
+       ExprContext *econtext;
+       int                     frameOptions = winstate->frameOptions;
+       Datum           value;
+       bool            isnull;
+       int16           len;
+       bool            byval;
+
+       /* Ensure we've not been called before for this scan */
+       Assert(winstate->all_first);
+
+       econtext = winstate->ss.ps.ps_ExprContext;
+
+       if (frameOptions & FRAMEOPTION_START_OFFSET)
+       {
+               Assert(winstate->startOffset != NULL);
+               value = ExecEvalExprSwitchContext(winstate->startOffset,
+                                                                                 econtext,
+                                                                                 &isnull);
+               if (isnull)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+                                        errmsg("frame starting offset must not be null")));
+               /* copy value into query-lifespan context */
+               get_typlenbyval(exprType((Node *) winstate->startOffset->expr),
+                                               &len,
+                                               &byval);
+               winstate->startOffsetValue = datumCopy(value, byval, len);
+               if (frameOptions & (FRAMEOPTION_ROWS | FRAMEOPTION_GROUPS))
+               {
+                       /* value is known to be int8 */
+                       int64           offset = DatumGetInt64(value);
+
+                       if (offset < 0)
+                               ereport(ERROR,
+                                               (errcode(ERRCODE_INVALID_PRECEDING_OR_FOLLOWING_SIZE),
+                                                errmsg("frame starting offset must not be negative")));
+               }
+       }
+
+       if (frameOptions & FRAMEOPTION_END_OFFSET)
+       {
+               Assert(winstate->endOffset != NULL);
+               value = ExecEvalExprSwitchContext(winstate->endOffset,
+                                                                                 econtext,
+                                                                                 &isnull);
+               if (isnull)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+                                        errmsg("frame ending offset must not be null")));
+               /* copy value into query-lifespan context */
+               get_typlenbyval(exprType((Node *) winstate->endOffset->expr),
+                                               &len,
+                                               &byval);
+               winstate->endOffsetValue = datumCopy(value, byval, len);
+               if (frameOptions & (FRAMEOPTION_ROWS | FRAMEOPTION_GROUPS))
+               {
+                       /* value is known to be int8 */
+                       int64           offset = DatumGetInt64(value);
+
+                       if (offset < 0)
+                               ereport(ERROR,
+                                               (errcode(ERRCODE_INVALID_PRECEDING_OR_FOLLOWING_SIZE),
+                                                errmsg("frame ending offset must not be negative")));
+               }
+       }
+       winstate->all_first = false;
+}
 
 /* -----------------
  * ExecWindowAgg
@@ -2061,68 +2137,8 @@ ExecWindowAgg(PlanState *pstate)
         * rescan).  These are assumed to hold constant throughout the scan; if
         * user gives us a volatile expression, we'll only use its initial value.
         */
-       if (winstate->all_first)
-       {
-               int                     frameOptions = winstate->frameOptions;
-               Datum           value;
-               bool            isnull;
-               int16           len;
-               bool            byval;
-
-               econtext = winstate->ss.ps.ps_ExprContext;
-
-               if (frameOptions & FRAMEOPTION_START_OFFSET)
-               {
-                       Assert(winstate->startOffset != NULL);
-                       value = ExecEvalExprSwitchContext(winstate->startOffset,
-                                                                                         econtext,
-                                                                                         &isnull);
-                       if (isnull)
-                               ereport(ERROR,
-                                               (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
-                                                errmsg("frame starting offset must not be null")));
-                       /* copy value into query-lifespan context */
-                       get_typlenbyval(exprType((Node *) winstate->startOffset->expr),
-                                                       &len, &byval);
-                       winstate->startOffsetValue = datumCopy(value, byval, len);
-                       if (frameOptions & (FRAMEOPTION_ROWS | FRAMEOPTION_GROUPS))
-                       {
-                               /* value is known to be int8 */
-                               int64           offset = DatumGetInt64(value);
-
-                               if (offset < 0)
-                                       ereport(ERROR,
-                                                       (errcode(ERRCODE_INVALID_PRECEDING_OR_FOLLOWING_SIZE),
-                                                        errmsg("frame starting offset must not be negative")));
-                       }
-               }
-               if (frameOptions & FRAMEOPTION_END_OFFSET)
-               {
-                       Assert(winstate->endOffset != NULL);
-                       value = ExecEvalExprSwitchContext(winstate->endOffset,
-                                                                                         econtext,
-                                                                                         &isnull);
-                       if (isnull)
-                               ereport(ERROR,
-                                               (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
-                                                errmsg("frame ending offset must not be null")));
-                       /* copy value into query-lifespan context */
-                       get_typlenbyval(exprType((Node *) winstate->endOffset->expr),
-                                                       &len, &byval);
-                       winstate->endOffsetValue = datumCopy(value, byval, len);
-                       if (frameOptions & (FRAMEOPTION_ROWS | FRAMEOPTION_GROUPS))
-                       {
-                               /* value is known to be int8 */
-                               int64           offset = DatumGetInt64(value);
-
-                               if (offset < 0)
-                                       ereport(ERROR,
-                                                       (errcode(ERRCODE_INVALID_PRECEDING_OR_FOLLOWING_SIZE),
-                                                        errmsg("frame ending offset must not be negative")));
-                       }
-               }
-               winstate->all_first = false;
-       }
+       if (unlikely(winstate->all_first))
+               calculate_frame_offsets(pstate);
 
        /* We need to loop as the runCondition or qual may filter out tuples */
        for (;;)