From d86e7c0172f1b390e55728f9a9ad92608b9f6320 Mon Sep 17 00:00:00 2001 From: jian he 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