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 (;;)