Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit bcc6c4c

Browse files
committed
Fix performance issue in EXPLAIN (ANALYZE, TIMING OFF).
Commit af7914c, which added the TIMING option to EXPLAIN, had an oversight: if the TIMING option is disabled then control in InstrStartNode() goes through an elog(DEBUG2) call, which typically does nothing but takes a noticeable amount of time to do it. Tweak the logic to avoid that. In HEAD, also change the elog(DEBUG2)'s in instrument.c to elog(ERROR). It's not very clear why they weren't like that to begin with, but this episode shows that not complaining more vociferously about misuse is likely to do little except allow bugs to remain hidden. While at it, adjust some code that was making possibly-dangerous assumptions about flag bits being in the rightmost byte of the instrument_options word. Problem reported by Pavel Stehule (via Tomas Vondra).
1 parent 9221f9d commit bcc6c4c

File tree

1 file changed

+11
-13
lines changed

1 file changed

+11
-13
lines changed

src/backend/executor/instrument.c

+11-13
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ InstrAlloc(int n, int instrument_options)
3333
instr = palloc0(n * sizeof(Instrumentation));
3434
if (instrument_options & (INSTRUMENT_BUFFERS | INSTRUMENT_TIMER))
3535
{
36+
bool need_buffers = (instrument_options & INSTRUMENT_BUFFERS) != 0;
37+
bool need_timer = (instrument_options & INSTRUMENT_TIMER) != 0;
3638
int i;
37-
bool need_buffers = instrument_options & INSTRUMENT_BUFFERS;
38-
bool need_timer = instrument_options & INSTRUMENT_TIMER;
3939

4040
for (i = 0; i < n; i++)
4141
{
@@ -51,10 +51,13 @@ InstrAlloc(int n, int instrument_options)
5151
void
5252
InstrStartNode(Instrumentation *instr)
5353
{
54-
if (instr->need_timer && INSTR_TIME_IS_ZERO(instr->starttime))
55-
INSTR_TIME_SET_CURRENT(instr->starttime);
56-
else
57-
elog(DEBUG2, "InstrStartNode called twice in a row");
54+
if (instr->need_timer)
55+
{
56+
if (INSTR_TIME_IS_ZERO(instr->starttime))
57+
INSTR_TIME_SET_CURRENT(instr->starttime);
58+
else
59+
elog(ERROR, "InstrStartNode called twice in a row");
60+
}
5861

5962
/* save buffer usage totals at node entry, if needed */
6063
if (instr->need_bufusage)
@@ -73,18 +76,13 @@ InstrStopNode(Instrumentation *instr, double nTuples)
7376
/* let's update the time only if the timer was requested */
7477
if (instr->need_timer)
7578
{
76-
7779
if (INSTR_TIME_IS_ZERO(instr->starttime))
78-
{
79-
elog(DEBUG2, "InstrStopNode called without start");
80-
return;
81-
}
80+
elog(ERROR, "InstrStopNode called without start");
8281

8382
INSTR_TIME_SET_CURRENT(endtime);
8483
INSTR_TIME_ACCUM_DIFF(instr->counter, endtime, instr->starttime);
8584

8685
INSTR_TIME_SET_ZERO(instr->starttime);
87-
8886
}
8987

9088
/* Add delta of buffer usage since entry to node's totals */
@@ -111,7 +109,7 @@ InstrEndLoop(Instrumentation *instr)
111109
return;
112110

113111
if (!INSTR_TIME_IS_ZERO(instr->starttime))
114-
elog(DEBUG2, "InstrEndLoop called on running node");
112+
elog(ERROR, "InstrEndLoop called on running node");
115113

116114
/* Accumulate per-cycle statistics into totals */
117115
totaltime = INSTR_TIME_GET_DOUBLE(instr->counter);

0 commit comments

Comments
 (0)