Hi. I always want to get rid of WindowAggState.nfaStateSize. It's ugly.
nfa_state_clone->nfa_state_make is hot path, but ExecRPRStartContext->nfa_state_make is not. In nfa_state_clone->nfa_state_make: memset(state, 0, winstate->nfaStateSize) is waste cycle, because nfa_state_clone would unconditionally memcpy from (int32 *counts) later. With that, we can get rid of memset in nfa_state_make conditionally, once memset is removed, nfaStateSize is no longer needed. Most of the row pattern code already works on WindowAggState, so refactoring makes the remaining rpr routines take WindowAggState instead of WindowObject as well. This looks more consistent. based on https://github.com/assam258-5892/postgres/tree/RPR-20260930 -- jian https://www.enterprisedb.com/
From d86e7c0172f1b390e55728f9a9ad92608b9f6320 Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Tue, 13 Oct 2026 11:10:03 +0800 Subject: [PATCH v54 3/3] Remove WindowAggState.nfaStateSize nfa_state_make() used it for memset(state, 0, winstate->nfaStateSize). nfa_state_clone()->nfa_state_make() on the hot path, so removing the cached size might have cost performance. Turns out the memset is not needed there at all: nfa_state_clone() overwrites every field right after. Once that unnecessary memset is gone, removing nfaStateSize has no effect on performance. Related: https://www.postgresql.org/message-id/CAAAe_zBi1dOtWb2vnwSvGwuU0-bqAOm_7dOM4u-CmukA8xaV5Q%40mail.gmail.com Related: https://www.postgresql.org/message-id/688312.1782222788%40sss.pgh.pa.us --- src/backend/executor/README.rpr | 2 -- src/backend/executor/execRPR.c | 36 ++++++++++++++++++---------- src/backend/executor/nodeWindowAgg.c | 2 -- src/include/nodes/execnodes.h | 1 - 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/src/backend/executor/README.rpr b/src/backend/executor/README.rpr index 16e158fdcf..1fac5b17ea 100644 --- a/src/backend/executor/README.rpr +++ b/src/backend/executor/README.rpr @@ -1234,7 +1234,6 @@ VI-3. RPR Fields of WindowAggState nfaVisitedEnds Nullable ENDs this DFS reached (cycle check) nfaVisitedMinWord Lowest bitmapword index touched since reset nfaVisitedMaxWord Highest bitmapword index touched since reset - nfaStateSize Precomputed size of RPRNFAState defineMatchStartDependent DEFINE vars needing per-context evaluation (match_start_dependent) nfaLastProcessedRow Last row processed by NFA (-1 = none) @@ -2833,7 +2832,6 @@ Appendix A. Data Structure Relationship Diagram |--- nfaVisitedMinWord / nfaVisitedMaxWord: int16 | (touched-word range for fast reset) |--- nfaLastProcessedRow: int64 (-1 = none) - |--- nfaStateSize: Size (pre-calculated RPRNFAState allocation size) |--- nfaContext <-> nfaContextTail (doubly-linked list) | +--- RPRNFAContext | |--- states: RPRNFAState* (linked list) diff --git a/src/backend/executor/execRPR.c b/src/backend/executor/execRPR.c index 9d8db1afc6..d19afd777d 100644 --- a/src/backend/executor/execRPR.c +++ b/src/backend/executor/execRPR.c @@ -75,7 +75,8 @@ nfa_mark_group_entered(WindowAggState *winstate, RPRPatternElement *begin) } /* Forward declarations */ -static RPRNFAState *nfa_state_make(WindowAggState *winstate); +static RPRNFAState *nfa_state_make(WindowAggState *winstate, + const int32 *counts); static void nfa_state_free(WindowAggState *winstate, RPRNFAState *state); static void nfa_state_free_list(WindowAggState *winstate, RPRNFAState *list); static RPRNFAState *nfa_state_clone(WindowAggState *winstate, int16 elemIdx, @@ -142,13 +143,16 @@ static void nfa_invalidate_dependent_vars(WindowAggState *winstate, /* * nfa_state_make * - * Allocate an NFA state, reusing from freeList if available. + * Allocate an NFA state on element 0, reusing from freeList if available. * freeList is stored in WindowAggState for reuse across match attempts. + * + * counts[] is copied from "counts", or set to all zero when that is NULL. */ static RPRNFAState * -nfa_state_make(WindowAggState *winstate) +nfa_state_make(WindowAggState *winstate, const int32 *counts) { RPRNFAState *state; + uint8 maxDepth = winstate->rpPattern->maxDepth; /* Try to reuse from free list first */ if (winstate->nfaStateFree != NULL) @@ -159,11 +163,21 @@ nfa_state_make(WindowAggState *winstate) else { /* Allocate in partition context for proper lifetime */ - state = MemoryContextAlloc(winstate->partcontext, winstate->nfaStateSize); + state = MemoryContextAlloc(winstate->partcontext, + offsetof(RPRNFAState, counts) + sizeof(int32) * (maxDepth)); } - /* Initialize entire state to zero */ - memset(state, 0, winstate->nfaStateSize); + state->next = NULL; + state->elemIdx = 0; + state->isAbsorbable = false; + + if (counts != NULL) + memcpy(state->counts, counts, sizeof(int32) * maxDepth); + else + { + for (int depth = 0; depth < maxDepth; depth++) + state->counts[depth] = 0; + } /* Update statistics */ winstate->nfaStatesActive++; @@ -223,14 +237,12 @@ nfa_state_clone(WindowAggState *winstate, int16 elemIdx, int32 *counts, bool sourceAbsorbable) { RPRPattern *pattern = winstate->rpPattern; - int maxDepth = pattern->maxDepth; - RPRNFAState *state = nfa_state_make(winstate); + RPRNFAState *state; RPRPatternElement *elem = &pattern->elements[elemIdx]; + Assert(counts != NULL); + state = nfa_state_make(winstate, counts); state->elemIdx = elemIdx; - /* Every reachable caller passes a live state's counts; maxDepth >= 1. */ - Assert(counts != NULL && maxDepth > 0); - memcpy(state->counts, counts, sizeof(int32) * maxDepth); /* * Compute isAbsorbable immediately at transition time. isAbsorbable = @@ -1754,7 +1766,7 @@ ExecRPRStartContext(WindowAggState *winstate, int64 startPos) ctx = nfa_context_make(winstate); ctx->matchStartRow = startPos; - ctx->states = nfa_state_make(winstate); /* initial state at elem 0 */ + ctx->states = nfa_state_make(winstate, NULL); /* * The only state so far sits on element 0, and computeAbsorbability() diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index df1f8864f5..b8e0bc7705 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -3120,8 +3120,6 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags) winstate->nfaContextTail = NULL; winstate->nfaContextFree = NULL; winstate->nfaStateFree = NULL; - winstate->nfaStateSize = offsetof(RPRNFAState, counts) + - sizeof(int32) * node->rpPattern->maxDepth; /* * Allocate the per-row varMatched cache. varNames are built in diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 40cf83b235..7893e9758c 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -2704,7 +2704,6 @@ typedef struct WindowAggState * traversal) */ RPRNFAContext *nfaContextFree; /* recycled NFA context nodes */ RPRNFAState *nfaStateFree; /* recycled NFA state nodes */ - Size nfaStateSize; /* pre-calculated RPRNFAState size */ RPRVarMatch *nfaVarMatched; /* per-row tri-state cache: varMatched[varId] * for varId < list_length(defineClauseExprs), * evaluated lazily */ -- 2.34.1
From e1ae1a2a207ff8151526448f7fcd3bb70a3193d8 Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Thu, 8 Oct 2026 18:55:59 +0800 Subject: [PATCH v54 1/3] Use WindowAggState instead of WindowObject in rpr routines Consistently using WindowAggState accross all of RPR routines. Most of RPR case, no need WindowObject. based on https://github.com/assam258-5892/postgres/tree/RPR-20260930 --- src/backend/executor/nodeWindowAgg.c | 34 ++++++++++++---------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index 01cec0f849..747c4d6a5c 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -250,17 +250,17 @@ static void put_notnull_info(WindowObject winobj, int64 pos, int argno, bool isnull); static bool rpr_is_defined(WindowAggState *winstate); static int64 row_is_in_reduced_frame(WindowObject winobj, int64 pos); -static void ensure_reduced_frame(WindowObject winobj, int64 pos); +static void ensure_reduced_frame(WindowAggState *winstate, int64 pos); static void clear_reduced_frame(WindowAggState *winstate); static int get_reduced_frame_status(WindowAggState *winstate, int64 pos); static void advance_nav_mark(WindowAggState *winstate, int64 currentPos); -static void advance_reduced_frame_nfa(WindowObject winobj, +static void advance_reduced_frame_nfa(WindowAggState *winstate, RPRNFAContext *targetCtx); -static void update_reduced_frame(WindowObject winobj, int64 pos); +static void update_reduced_frame(WindowAggState *winstate, int64 pos); /* Forward declarations - DEFINE row evaluation */ -static bool rpr_prepare_row(WindowObject winobj, int64 pos, RPRVarMatch *varMatched); +static bool rpr_prepare_row(WindowAggState *winstate, int64 pos, RPRVarMatch *varMatched); static void build_define_offsets(WindowAggState *winstate, List *defineClause); static void resolve_nav_offsets(WindowAggState *winstate); static void resolve_one_nav(RPRNavOffsets *entry, EvalDefineOffsetsContext *context); @@ -2586,8 +2586,7 @@ ExecWindowAgg(PlanState *pstate) * leave the match state behind currentpos. */ Assert(winstate->nav_winobj != NULL); - ensure_reduced_frame(winstate->nav_winobj, - winstate->currentpos); + ensure_reduced_frame(winstate, winstate->currentpos); } /* @@ -4496,7 +4495,7 @@ row_is_in_reduced_frame(WindowObject winobj, int64 pos) return rtn; } - ensure_reduced_frame(winobj, pos); + ensure_reduced_frame(winstate, pos); state = get_reduced_frame_status(winstate, pos); @@ -4533,14 +4532,12 @@ row_is_in_reduced_frame(WindowObject winobj, int64 pos) * scan, and again when a window function accesses the frame). */ static void -ensure_reduced_frame(WindowObject winobj, int64 pos) +ensure_reduced_frame(WindowAggState *winstate, int64 pos) { - WindowAggState *winstate = winobj->winstate; - if (get_reduced_frame_status(winstate, pos) == RF_NOT_DETERMINED) { update_frameheadpos(winstate); - update_reduced_frame(winobj, pos); + update_reduced_frame(winstate, pos); } } @@ -4669,9 +4666,8 @@ advance_nav_mark(WindowAggState *winstate, int64 currentPos) * evaluations are shared across all active contexts. */ static void -advance_reduced_frame_nfa(WindowObject winobj, RPRNFAContext *targetCtx) +advance_reduced_frame_nfa(WindowAggState *winstate, RPRNFAContext *targetCtx) { - WindowAggState *winstate = winobj->winstate; int64 currentPos; int64 startPos; int64 saved_currentpos = winstate->currentpos; @@ -4710,7 +4706,7 @@ advance_reduced_frame_nfa(WindowObject winobj, RPRNFAContext *targetCtx) winstate->nav_match_start = targetCtx->matchStartRow; /* No more rows in partition? Finalize all contexts */ - if (!rpr_prepare_row(winobj, currentPos, winstate->nfaVarMatched)) + if (!rpr_prepare_row(winstate, currentPos, winstate->nfaVarMatched)) { ExecRPRFinalizeAllContexts(winstate, currentPos - 1); /* Clean up dead contexts from finalization */ @@ -4765,9 +4761,8 @@ advance_reduced_frame_nfa(WindowObject winobj, RPRNFAContext *targetCtx) * - Contexts persist across calls, enabling O(n) DEFINE evaluations */ static void -update_reduced_frame(WindowObject winobj, int64 pos) +update_reduced_frame(WindowAggState *winstate, int64 pos) { - WindowAggState *winstate = winobj->winstate; RPRNFAContext *targetCtx = NULL; winstate->rpr_match_start = pos; @@ -4822,7 +4817,7 @@ update_reduced_frame(WindowObject winobj, int64 pos) * is registered below. */ if (targetCtx->states != NULL) - advance_reduced_frame_nfa(winobj, targetCtx); + advance_reduced_frame_nfa(winstate, targetCtx); if (targetCtx->matchedState == NULL) { @@ -4872,15 +4867,14 @@ update_reduced_frame(WindowObject winobj, int64 pos) * opcodes during expression evaluation, which temporarily swap the slot. */ static bool -rpr_prepare_row(WindowObject winobj, int64 pos, RPRVarMatch *varMatched) +rpr_prepare_row(WindowAggState *winstate, int64 pos, RPRVarMatch *varMatched) { - WindowAggState *winstate = winobj->winstate; ExprContext *econtext = winstate->rprContext; TupleTableSlot *slot; /* Fetch current row into temp_slot_1 */ slot = winstate->temp_slot_1; - if (!window_gettupleslot(winobj, pos, slot)) + if (!window_gettupleslot(winstate->nav_winobj, pos, slot)) return false; /* No row exists */ /* Set up 1-slot context: only ecxt_outertuple */ -- 2.34.1
From 849ed0dcf5110c2fc66b06dd6ca52dc6dbcd84ed Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Tue, 13 Oct 2026 11:19:15 +0800 Subject: [PATCH v54 2/3] minor, using palloc0_array instead of palloc0 --- src/backend/executor/nodeWindowAgg.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index 747c4d6a5c..df1f8864f5 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -3133,8 +3133,8 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags) * rpr_prepare_row() can reset the cache unconditionally. */ Assert(winstate->defineClauseExprs != NIL); - winstate->nfaVarMatched = palloc0(sizeof(RPRVarMatch) * - list_length(winstate->defineClauseExprs)); + winstate->nfaVarMatched = palloc0_array(RPRVarMatch, + list_length(winstate->defineClauseExprs)); /* Copy match_start dependency bitmapset for per-context evaluation */ winstate->defineMatchStartDependent = bms_copy(node->defineMatchStartDependent); @@ -3142,8 +3142,7 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags) nfaVisitedNWords = (node->rpPattern->numElements - 1) / BITS_PER_BITMAPWORD + 1; - winstate->nfaVisitedEnds = palloc0(sizeof(bitmapword) * - nfaVisitedNWords); + winstate->nfaVisitedEnds = palloc0_array(bitmapword, nfaVisitedNWords); /* High-water mark sentinels: no bits set yet. */ winstate->nfaVisitedMinWord = PG_INT16_MAX; -- 2.34.1
