Re: [PATCH] Avoid unnecessary code execution in Instrument.c when TIMING is FALSE

On 2025-07-26 00:26, Hironobu SUZUKI wrote:
Thanks for the proposal!

> Hi,
>
> Even when using EXPLAIN ANALYZE with TIMING=FALSE, the functions
> InstrStopNode(), InstrEndLoop(), and InstrAggNode() in Instrument.c
> still execute code related to the "starttime", "counter",
> "firsttuple", "startup", and "total" fields within the
> Instrumentation structure.
> These operations are unnecessary when timing is disabled, and since
> these functions are called very frequently, I have created a patch to
> address this.

Is the main purpose of this patch to reduce unnecessary performance
overhead? Or is it rather to make it explicit that these values are not
retrieved when the timing option is off?
Since you mentioned that "these functions are called very frequently", I
assume the former is your main concern.

During the meetup, I had understood that even with 'timing' disabled,
there were still costly operations such as calls to functions that
retrieve timestamps.
However, when I looked at the patch, I couldn't find operations that
looked particularly expensive, so I'd like to confirm this.

- instr->firsttuple = INSTR_TIME_GET_DOUBLE(instr->counter);
+ if (instr->need_timer)
+ instr->firsttuple = INSTR_TIME_GET_DOUBLE(instr->counter);

For example, if 'timing' is off, instr->counter would just be zero, and
INSTR_TIME_GET_DOUBLE() is a macro that does not seem to perform any
heavy operation.
Given that, I'm unsure whether the added if statement provides a net
performance benefit.

If there is indeed a measurable performance overhead, could you please
share an example workload where performance improves before and after
applying the patch?

Regards,

--
Atsushi Torikoshi
Seconded from NTT DATA Japan Corporation to SRA OSS K.K.