Remove volatile qualifiers from pg_stat_statements.c.

Prior to commit 0709b7ee72, which changed the spinlock primitives
to function as compiler barriers, access to variables within a
spinlock-protected section required using a volatile pointer, but
that is no longer necessary.

Reviewed-by: Bertrand Drouvot, Michael Paquier
Discussion: https://postgr.es/m/Zqkv9iK7MkNS0KaN%40nathan
pull/173/head
Nathan Bossart 11 months ago
parent 3dcb09de7b
commit 8928817769
  1. 229
      contrib/pg_stat_statements/pg_stat_statements.c

@ -301,10 +301,9 @@ static bool pgss_save = true; /* whether to save stats across shutdown */
#define record_gc_qtexts() \ #define record_gc_qtexts() \
do { \ do { \
volatile pgssSharedState *s = (volatile pgssSharedState *) pgss; \ SpinLockAcquire(&pgss->mutex); \
SpinLockAcquire(&s->mutex); \ pgss->gc_count++; \
s->gc_count++; \ SpinLockRelease(&pgss->mutex); \
SpinLockRelease(&s->mutex); \
} while(0) } while(0)
/*---- Function declarations ----*/ /*---- Function declarations ----*/
@ -1386,28 +1385,26 @@ pgss_store(const char *query, uint64 queryId,
/* Increment the counts, except when jstate is not NULL */ /* Increment the counts, except when jstate is not NULL */
if (!jstate) if (!jstate)
{ {
Assert(kind == PGSS_PLAN || kind == PGSS_EXEC);
/* /*
* Grab the spinlock while updating the counters (see comment about * Grab the spinlock while updating the counters (see comment about
* locking rules at the head of the file) * locking rules at the head of the file)
*/ */
volatile pgssEntry *e = (volatile pgssEntry *) entry; SpinLockAcquire(&entry->mutex);
Assert(kind == PGSS_PLAN || kind == PGSS_EXEC);
SpinLockAcquire(&e->mutex);
/* "Unstick" entry if it was previously sticky */ /* "Unstick" entry if it was previously sticky */
if (IS_STICKY(e->counters)) if (IS_STICKY(entry->counters))
e->counters.usage = USAGE_INIT; entry->counters.usage = USAGE_INIT;
e->counters.calls[kind] += 1; entry->counters.calls[kind] += 1;
e->counters.total_time[kind] += total_time; entry->counters.total_time[kind] += total_time;
if (e->counters.calls[kind] == 1) if (entry->counters.calls[kind] == 1)
{ {
e->counters.min_time[kind] = total_time; entry->counters.min_time[kind] = total_time;
e->counters.max_time[kind] = total_time; entry->counters.max_time[kind] = total_time;
e->counters.mean_time[kind] = total_time; entry->counters.mean_time[kind] = total_time;
} }
else else
{ {
@ -1415,75 +1412,75 @@ pgss_store(const char *query, uint64 queryId,
* Welford's method for accurately computing variance. See * Welford's method for accurately computing variance. See
* <http://www.johndcook.com/blog/standard_deviation/> * <http://www.johndcook.com/blog/standard_deviation/>
*/ */
double old_mean = e->counters.mean_time[kind]; double old_mean = entry->counters.mean_time[kind];
e->counters.mean_time[kind] += entry->counters.mean_time[kind] +=
(total_time - old_mean) / e->counters.calls[kind]; (total_time - old_mean) / entry->counters.calls[kind];
e->counters.sum_var_time[kind] += entry->counters.sum_var_time[kind] +=
(total_time - old_mean) * (total_time - e->counters.mean_time[kind]); (total_time - old_mean) * (total_time - entry->counters.mean_time[kind]);
/* /*
* Calculate min and max time. min = 0 and max = 0 means that the * Calculate min and max time. min = 0 and max = 0 means that the
* min/max statistics were reset * min/max statistics were reset
*/ */
if (e->counters.min_time[kind] == 0 if (entry->counters.min_time[kind] == 0
&& e->counters.max_time[kind] == 0) && entry->counters.max_time[kind] == 0)
{ {
e->counters.min_time[kind] = total_time; entry->counters.min_time[kind] = total_time;
e->counters.max_time[kind] = total_time; entry->counters.max_time[kind] = total_time;
} }
else else
{ {
if (e->counters.min_time[kind] > total_time) if (entry->counters.min_time[kind] > total_time)
e->counters.min_time[kind] = total_time; entry->counters.min_time[kind] = total_time;
if (e->counters.max_time[kind] < total_time) if (entry->counters.max_time[kind] < total_time)
e->counters.max_time[kind] = total_time; entry->counters.max_time[kind] = total_time;
} }
} }
e->counters.rows += rows; entry->counters.rows += rows;
e->counters.shared_blks_hit += bufusage->shared_blks_hit; entry->counters.shared_blks_hit += bufusage->shared_blks_hit;
e->counters.shared_blks_read += bufusage->shared_blks_read; entry->counters.shared_blks_read += bufusage->shared_blks_read;
e->counters.shared_blks_dirtied += bufusage->shared_blks_dirtied; entry->counters.shared_blks_dirtied += bufusage->shared_blks_dirtied;
e->counters.shared_blks_written += bufusage->shared_blks_written; entry->counters.shared_blks_written += bufusage->shared_blks_written;
e->counters.local_blks_hit += bufusage->local_blks_hit; entry->counters.local_blks_hit += bufusage->local_blks_hit;
e->counters.local_blks_read += bufusage->local_blks_read; entry->counters.local_blks_read += bufusage->local_blks_read;
e->counters.local_blks_dirtied += bufusage->local_blks_dirtied; entry->counters.local_blks_dirtied += bufusage->local_blks_dirtied;
e->counters.local_blks_written += bufusage->local_blks_written; entry->counters.local_blks_written += bufusage->local_blks_written;
e->counters.temp_blks_read += bufusage->temp_blks_read; entry->counters.temp_blks_read += bufusage->temp_blks_read;
e->counters.temp_blks_written += bufusage->temp_blks_written; entry->counters.temp_blks_written += bufusage->temp_blks_written;
e->counters.shared_blk_read_time += INSTR_TIME_GET_MILLISEC(bufusage->shared_blk_read_time); entry->counters.shared_blk_read_time += INSTR_TIME_GET_MILLISEC(bufusage->shared_blk_read_time);
e->counters.shared_blk_write_time += INSTR_TIME_GET_MILLISEC(bufusage->shared_blk_write_time); entry->counters.shared_blk_write_time += INSTR_TIME_GET_MILLISEC(bufusage->shared_blk_write_time);
e->counters.local_blk_read_time += INSTR_TIME_GET_MILLISEC(bufusage->local_blk_read_time); entry->counters.local_blk_read_time += INSTR_TIME_GET_MILLISEC(bufusage->local_blk_read_time);
e->counters.local_blk_write_time += INSTR_TIME_GET_MILLISEC(bufusage->local_blk_write_time); entry->counters.local_blk_write_time += INSTR_TIME_GET_MILLISEC(bufusage->local_blk_write_time);
e->counters.temp_blk_read_time += INSTR_TIME_GET_MILLISEC(bufusage->temp_blk_read_time); entry->counters.temp_blk_read_time += INSTR_TIME_GET_MILLISEC(bufusage->temp_blk_read_time);
e->counters.temp_blk_write_time += INSTR_TIME_GET_MILLISEC(bufusage->temp_blk_write_time); entry->counters.temp_blk_write_time += INSTR_TIME_GET_MILLISEC(bufusage->temp_blk_write_time);
e->counters.usage += USAGE_EXEC(total_time); entry->counters.usage += USAGE_EXEC(total_time);
e->counters.wal_records += walusage->wal_records; entry->counters.wal_records += walusage->wal_records;
e->counters.wal_fpi += walusage->wal_fpi; entry->counters.wal_fpi += walusage->wal_fpi;
e->counters.wal_bytes += walusage->wal_bytes; entry->counters.wal_bytes += walusage->wal_bytes;
if (jitusage) if (jitusage)
{ {
e->counters.jit_functions += jitusage->created_functions; entry->counters.jit_functions += jitusage->created_functions;
e->counters.jit_generation_time += INSTR_TIME_GET_MILLISEC(jitusage->generation_counter); entry->counters.jit_generation_time += INSTR_TIME_GET_MILLISEC(jitusage->generation_counter);
if (INSTR_TIME_GET_MILLISEC(jitusage->deform_counter)) if (INSTR_TIME_GET_MILLISEC(jitusage->deform_counter))
e->counters.jit_deform_count++; entry->counters.jit_deform_count++;
e->counters.jit_deform_time += INSTR_TIME_GET_MILLISEC(jitusage->deform_counter); entry->counters.jit_deform_time += INSTR_TIME_GET_MILLISEC(jitusage->deform_counter);
if (INSTR_TIME_GET_MILLISEC(jitusage->inlining_counter)) if (INSTR_TIME_GET_MILLISEC(jitusage->inlining_counter))
e->counters.jit_inlining_count++; entry->counters.jit_inlining_count++;
e->counters.jit_inlining_time += INSTR_TIME_GET_MILLISEC(jitusage->inlining_counter); entry->counters.jit_inlining_time += INSTR_TIME_GET_MILLISEC(jitusage->inlining_counter);
if (INSTR_TIME_GET_MILLISEC(jitusage->optimization_counter)) if (INSTR_TIME_GET_MILLISEC(jitusage->optimization_counter))
e->counters.jit_optimization_count++; entry->counters.jit_optimization_count++;
e->counters.jit_optimization_time += INSTR_TIME_GET_MILLISEC(jitusage->optimization_counter); entry->counters.jit_optimization_time += INSTR_TIME_GET_MILLISEC(jitusage->optimization_counter);
if (INSTR_TIME_GET_MILLISEC(jitusage->emission_counter)) if (INSTR_TIME_GET_MILLISEC(jitusage->emission_counter))
e->counters.jit_emission_count++; entry->counters.jit_emission_count++;
e->counters.jit_emission_time += INSTR_TIME_GET_MILLISEC(jitusage->emission_counter); entry->counters.jit_emission_time += INSTR_TIME_GET_MILLISEC(jitusage->emission_counter);
} }
SpinLockRelease(&e->mutex); SpinLockRelease(&entry->mutex);
} }
done: done:
@ -1724,15 +1721,11 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
int n_writers; int n_writers;
/* Take the mutex so we can examine variables */ /* Take the mutex so we can examine variables */
{ SpinLockAcquire(&pgss->mutex);
volatile pgssSharedState *s = (volatile pgssSharedState *) pgss; extent = pgss->extent;
n_writers = pgss->n_writers;
SpinLockAcquire(&s->mutex); gc_count = pgss->gc_count;
extent = s->extent; SpinLockRelease(&pgss->mutex);
n_writers = s->n_writers;
gc_count = s->gc_count;
SpinLockRelease(&s->mutex);
}
/* No point in loading file now if there are active writers */ /* No point in loading file now if there are active writers */
if (n_writers == 0) if (n_writers == 0)
@ -1847,15 +1840,11 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
} }
/* copy counters to a local variable to keep locking time short */ /* copy counters to a local variable to keep locking time short */
{ SpinLockAcquire(&entry->mutex);
volatile pgssEntry *e = (volatile pgssEntry *) entry; tmp = entry->counters;
stats_since = entry->stats_since;
SpinLockAcquire(&e->mutex); minmax_stats_since = entry->minmax_stats_since;
tmp = e->counters; SpinLockRelease(&entry->mutex);
stats_since = e->stats_since;
minmax_stats_since = e->minmax_stats_since;
SpinLockRelease(&e->mutex);
}
/* Skip entry if unexecuted (ie, it's a pending "sticky" entry) */ /* Skip entry if unexecuted (ie, it's a pending "sticky" entry) */
if (IS_STICKY(tmp)) if (IS_STICKY(tmp))
@ -1996,13 +1985,9 @@ pg_stat_statements_info(PG_FUNCTION_ARGS)
elog(ERROR, "return type must be a row type"); elog(ERROR, "return type must be a row type");
/* Read global statistics for pg_stat_statements */ /* Read global statistics for pg_stat_statements */
{ SpinLockAcquire(&pgss->mutex);
volatile pgssSharedState *s = (volatile pgssSharedState *) pgss; stats = pgss->stats;
SpinLockRelease(&pgss->mutex);
SpinLockAcquire(&s->mutex);
stats = s->stats;
SpinLockRelease(&s->mutex);
}
values[0] = Int64GetDatum(stats.dealloc); values[0] = Int64GetDatum(stats.dealloc);
values[1] = TimestampTzGetDatum(stats.stats_reset); values[1] = TimestampTzGetDatum(stats.stats_reset);
@ -2169,13 +2154,9 @@ entry_dealloc(void)
pfree(entries); pfree(entries);
/* Increment the number of times entries are deallocated */ /* Increment the number of times entries are deallocated */
{ SpinLockAcquire(&pgss->mutex);
volatile pgssSharedState *s = (volatile pgssSharedState *) pgss; pgss->stats.dealloc += 1;
SpinLockRelease(&pgss->mutex);
SpinLockAcquire(&s->mutex);
s->stats.dealloc += 1;
SpinLockRelease(&s->mutex);
}
} }
/* /*
@ -2205,17 +2186,13 @@ qtext_store(const char *query, int query_len,
* We use a spinlock to protect extent/n_writers/gc_count, so that * We use a spinlock to protect extent/n_writers/gc_count, so that
* multiple processes may execute this function concurrently. * multiple processes may execute this function concurrently.
*/ */
{ SpinLockAcquire(&pgss->mutex);
volatile pgssSharedState *s = (volatile pgssSharedState *) pgss; off = pgss->extent;
pgss->extent += query_len + 1;
SpinLockAcquire(&s->mutex); pgss->n_writers++;
off = s->extent; if (gc_count)
s->extent += query_len + 1; *gc_count = pgss->gc_count;
s->n_writers++; SpinLockRelease(&pgss->mutex);
if (gc_count)
*gc_count = s->gc_count;
SpinLockRelease(&s->mutex);
}
*query_offset = off; *query_offset = off;
@ -2244,13 +2221,9 @@ qtext_store(const char *query, int query_len,
CloseTransientFile(fd); CloseTransientFile(fd);
/* Mark our write complete */ /* Mark our write complete */
{ SpinLockAcquire(&pgss->mutex);
volatile pgssSharedState *s = (volatile pgssSharedState *) pgss; pgss->n_writers--;
SpinLockRelease(&pgss->mutex);
SpinLockAcquire(&s->mutex);
s->n_writers--;
SpinLockRelease(&s->mutex);
}
return true; return true;
@ -2264,13 +2237,9 @@ error:
CloseTransientFile(fd); CloseTransientFile(fd);
/* Mark our write complete */ /* Mark our write complete */
{ SpinLockAcquire(&pgss->mutex);
volatile pgssSharedState *s = (volatile pgssSharedState *) pgss; pgss->n_writers--;
SpinLockRelease(&pgss->mutex);
SpinLockAcquire(&s->mutex);
s->n_writers--;
SpinLockRelease(&s->mutex);
}
return false; return false;
} }
@ -2408,13 +2377,9 @@ need_gc_qtexts(void)
Size extent; Size extent;
/* Read shared extent pointer */ /* Read shared extent pointer */
{ SpinLockAcquire(&pgss->mutex);
volatile pgssSharedState *s = (volatile pgssSharedState *) pgss; extent = pgss->extent;
SpinLockRelease(&pgss->mutex);
SpinLockAcquire(&s->mutex);
extent = s->extent;
SpinLockRelease(&s->mutex);
}
/* /*
* Don't proceed if file does not exceed 512 bytes per possible entry. * Don't proceed if file does not exceed 512 bytes per possible entry.
@ -2733,14 +2698,10 @@ entry_reset(Oid userid, Oid dbid, uint64 queryid, bool minmax_only)
* Reset global statistics for pg_stat_statements since all entries are * Reset global statistics for pg_stat_statements since all entries are
* removed. * removed.
*/ */
{ SpinLockAcquire(&pgss->mutex);
volatile pgssSharedState *s = (volatile pgssSharedState *) pgss; pgss->stats.dealloc = 0;
pgss->stats.stats_reset = stats_reset;
SpinLockAcquire(&s->mutex); SpinLockRelease(&pgss->mutex);
s->stats.dealloc = 0;
s->stats.stats_reset = stats_reset;
SpinLockRelease(&s->mutex);
}
/* /*
* Write new empty query file, perhaps even creating a new one to recover * Write new empty query file, perhaps even creating a new one to recover

Loading…
Cancel
Save