From 886ab44711541812ddf66788a84e924024d7dd5c Mon Sep 17 00:00:00 2001 From: Lukas Fittl Date: Sun, 5 Apr 2026 19:30:56 -0700 Subject: [PATCH v18 6/9] instrumentation: Specialize ExecProcNodeInstr for each set of options For most queries, the bulk of the overhead of EXPLAIN ANALYZE happens in ExecProcNodeInstr when starting/stopping instrumentation for that node. Previously each ExecProcNodeInstr would check which instrumentation options are active in the InstrStartNode/InstrStopNode calls, and do the corresponding work (timers, instrumentation stack, etc.). These conditionals being checked for each tuple being emitted add up, and cause a non-optimal set of instructions to be generated by the compiler. Because we already have a mechanism to specify a function pointer when instrumentation is enabled, we can instead create specialized functions that are tailored to the instrumentation options enabled, and avoid conditionals on subsequent ExecProcNodeInstr calls. In a stress test with a large COUNT(*) that does many ExecProcNode calls, this roughly halves the overhead of EXPLAIN (ANALYZE, TIMING OFF, BUFFERS OFF), from ~13% on top of actual runtime to ~6%. When using BUFFERS ON the same query goes from ~20% to ~15% overhead. Author: Lukas Fittl Reviewed-by: Zsolt Parragi Discussion: https://postgr.es/m/CAP53PkzdBK8VJ1fS4AZ481LgMN8f9mJiC39ZRHqkFUSYq6KWmg%40mail.gmail.com --- src/backend/executor/execProcnode.c | 2 +- src/backend/executor/instrument.c | 180 +++++++++++++++++++++------- src/include/executor/executor.h | 5 +- 3 files changed, 141 insertions(+), 46 deletions(-) diff --git a/src/backend/executor/execProcnode.c b/src/backend/executor/execProcnode.c index 5ca8d91344b..357175ac2cb 100644 --- a/src/backend/executor/execProcnode.c +++ b/src/backend/executor/execProcnode.c @@ -465,7 +465,7 @@ ExecProcNodeFirst(PlanState *node) * have ExecProcNode() directly call the relevant function from now on. */ if (node->instrument) - node->ExecProcNode = ExecProcNodeInstr; + node->ExecProcNode = InstrNodeSetupExecProcNode(node->instrument); else node->ExecProcNode = node->ExecProcNodeReal; diff --git a/src/backend/executor/instrument.c b/src/backend/executor/instrument.c index e5580395e8b..db93562a8fc 100644 --- a/src/backend/executor/instrument.c +++ b/src/backend/executor/instrument.c @@ -74,19 +74,27 @@ InstrInitOptions(Instrumentation *instr, int instrument_options) instr->need_timer = (instrument_options & INSTRUMENT_TIMER) != 0; } -inline void -InstrStart(Instrumentation *instr) +static inline void +InstrStartTimer(Instrumentation *instr) { - if (instr->need_timer) - { - if (!INSTR_TIME_IS_ZERO(instr->starttime)) - elog(ERROR, "InstrStart called twice in a row"); - else - INSTR_TIME_SET_CURRENT_FAST(instr->starttime); - } + if (!INSTR_TIME_IS_ZERO(instr->starttime)) + elog(ERROR, "InstrStart called twice in a row"); - if (instr->need_stack) - InstrPushStack(instr); + INSTR_TIME_SET_CURRENT_FAST(instr->starttime); +} + +static inline void +InstrStopTimer(Instrumentation *instr, instr_time *accum_time) +{ + instr_time endtime; + + if (INSTR_TIME_IS_ZERO(instr->starttime)) + elog(ERROR, "InstrStop called without start"); + + INSTR_TIME_SET_CURRENT_FAST(endtime); + INSTR_TIME_ACCUM_DIFF(*accum_time, endtime, instr->starttime); + + INSTR_TIME_SET_ZERO(instr->starttime); } /* @@ -96,25 +104,25 @@ InstrStart(Instrumentation *instr) static inline void InstrStopCommon(Instrumentation *instr, instr_time *accum_time) { - instr_time endtime; - /* update the time only if the timer was requested */ if (instr->need_timer) - { - if (INSTR_TIME_IS_ZERO(instr->starttime)) - elog(ERROR, "InstrStop called without start"); - - INSTR_TIME_SET_CURRENT_FAST(endtime); - INSTR_TIME_ACCUM_DIFF(*accum_time, endtime, instr->starttime); - - INSTR_TIME_SET_ZERO(instr->starttime); - } + InstrStopTimer(instr, accum_time); /* pop the stack, unless InstrStopFinalize previously cleaned up */ if (instr->on_stack) InstrPopStack(instr); } +void +InstrStart(Instrumentation *instr) +{ + if (instr->need_timer) + InstrStartTimer(instr); + + if (instr->need_stack) + InstrPushStack(instr); +} + void InstrStop(Instrumentation *instr) { @@ -462,32 +470,30 @@ InstrInitNode(NodeInstrumentation *instr, int instrument_options, bool async_mod instr->async_mode = async_mode; } -/* Entry to a plan node */ -inline void +/* Entry to a plan node. If you modify this, check InstrNodeSetupExecProcNode. */ +void InstrStartNode(NodeInstrumentation *instr) { InstrStart(&instr->instr); } - -/* Exit from a plan node */ -inline void -InstrStopNode(NodeInstrumentation *instr, double nTuples) +/* + * Count the tuples returned by a plan node call, and remember the time it + * took to get to the first tuple of this cycle (which InstrEndLoop turns into + * the node's startup time). Must be called after the node's timer has been + * stopped for this call. + * + * Shared by InstrStopNode and the specialized ExecProcNode wrappers below, + * so that both account in exactly the same way. + */ +static inline void +InstrCountNodeTuples(NodeInstrumentation *instr, double nTuples) { double save_tuplecount = instr->tuplecount; /* count the returned tuples */ instr->tuplecount += nTuples; - /* - * Note that in contrast to InstrStop() the time is accumulated into - * NodeInstrumentation->counter, with total only getting updated in - * InstrEndLoop. We need the separate counter variable because we need to - * calculate start-up time for the first tuple in each cycle, and then - * accumulate it together. - */ - InstrStopCommon(&instr->instr, &instr->counter); - /* Is this the first tuple of this cycle? */ if (!instr->running) { @@ -505,28 +511,116 @@ InstrStopNode(NodeInstrumentation *instr, double nTuples) } } +/* Exit from a plan node */ +void +InstrStopNode(NodeInstrumentation *instr, double nTuples) +{ + /* + * Note that in contrast to InstrStop() the time is accumulated into + * NodeInstrumentation->counter, with total only getting updated in + * InstrEndLoop. We need the separate counter variable because we need to + * calculate start-up time for the first tuple in each cycle, and then + * accumulate it together. + */ + InstrStopCommon(&instr->instr, &instr->counter); + + InstrCountNodeTuples(instr, nTuples); +} + /* - * ExecProcNode wrapper that performs instrumentation calls. By keeping - * this a separate function, we avoid overhead in the normal case where + * ExecProcNode wrappers that perform instrumentation calls. By keeping + * them in separate functions, we avoid overhead in the normal case where * no instrumentation is wanted. * + * These functions are equivalent to running ExecProcNodeReal wrapped in + * InstrStartNode and InstrStopNode, but avoid the conditionals in the hot path + * by checking the instrumentation options when the ExecProcNode pointer gets + * first set, and then using a special-purpose function for each. This results + * in a more optimized set of compiled instructions. + * * This is implemented in instrument.c as all the functions it calls directly * are here, allowing them to be inlined even when not using LTO. */ -TupleTableSlot * -ExecProcNodeInstr(PlanState *node) + +/* Simplified pop: restore saved state instead of re-deriving from array */ +static inline void +InstrPopStackTo(Instrumentation *prev) +{ + Assert(instr_stack.stack_size > 0); + Assert(instr_stack.stack_size > 1 ? instr_stack.entries[instr_stack.stack_size - 2] == prev : &instr_top == prev); + instr_stack.entries[instr_stack.stack_size - 1]->on_stack = false; + instr_stack.stack_size--; + instr_stack.current = prev; +} + +static pg_always_inline TupleTableSlot * +ExecProcNodeInstr(PlanState *node, bool need_timer, bool need_stack) { + NodeInstrumentation *instr = node->instrument; + Instrumentation *prev = instr_stack.current; TupleTableSlot *result; - InstrStartNode(node->instrument); + if (need_stack) + InstrPushStack(&instr->instr); + if (need_timer) + InstrStartTimer(&instr->instr); result = node->ExecProcNodeReal(node); - InstrStopNode(node->instrument, TupIsNull(result) ? 0.0 : 1.0); + if (need_timer) + InstrStopTimer(&instr->instr, &instr->counter); + if (need_stack) + InstrPopStackTo(prev); + + InstrCountNodeTuples(instr, TupIsNull(result) ? 0.0 : 1.0); return result; } +static TupleTableSlot * +ExecProcNodeInstrFull(PlanState *node) +{ + return ExecProcNodeInstr(node, true, true); +} + +static TupleTableSlot * +ExecProcNodeInstrRowsStackOnly(PlanState *node) +{ + return ExecProcNodeInstr(node, false, true); +} + +static TupleTableSlot * +ExecProcNodeInstrRowsTimerOnly(PlanState *node) +{ + return ExecProcNodeInstr(node, true, false); +} + +static TupleTableSlot * +ExecProcNodeInstrRowsOnly(PlanState *node) +{ + return ExecProcNodeInstr(node, false, false); +} + +/* + * Returns an ExecProcNode wrapper that performs instrumentation calls, + * tailored to the instrumentation options enabled for the node. + */ +ExecProcNodeMtd +InstrNodeSetupExecProcNode(NodeInstrumentation *instr) +{ + bool need_timer = instr->instr.need_timer; + bool need_stack = instr->instr.need_stack; + + if (need_timer && need_stack) + return ExecProcNodeInstrFull; + else if (need_stack) + return ExecProcNodeInstrRowsStackOnly; + else if (need_timer) + return ExecProcNodeInstrRowsTimerOnly; + else + return ExecProcNodeInstrRowsOnly; +} + /* Update tuple count */ void InstrUpdateTupleCount(NodeInstrumentation *instr, double nTuples) diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index e34dd2c8054..f6fad6a6e68 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -308,11 +308,12 @@ extern void ExecFinalizeWorkerInstrumentation(PlanState *node); extern void ExecSetTupleBound(int64 tuples_needed, PlanState *child_node); /* - * ExecProcNodeInstr() is implemented in instrument.c, as that allows for + * InstrNodeSetupExecProcNode() is implemented in instrument.c, as that allows for * inlining of the instrumentation functions, but thematically it ought to be * in execProcnode.c. */ -extern TupleTableSlot *ExecProcNodeInstr(PlanState *node); + +extern ExecProcNodeMtd InstrNodeSetupExecProcNode(NodeInstrumentation *instr); /* ---------------------------------------------------------------- -- 2.47.1