pg_stat_statements: Avoid some locking during PGSS entry scans

A single PGSS entry's spinlock is used to be able to modify "counters"
without holding pgss->lock exclusively, as mentioned at the top of
pg_stat_statements.c and within pgssEntry.

Within a single pgssEntry, stats_since and minmax_stats_since are never
modified without holding pgss->lock exclusively, so there is no need to
hold an entry's spinlock when reading stats_since and
minmax_stats_since, as done when scanning all the PGSS entries for
function calls of pg_stat_statements().

This also restores the consistency between the code and the comments
about the entry's spinlock usage.  This change is a performance
improvement (it can be argued that this is a logic bug), so there is no
need for a backpatch.  This saves two instructions from being read while
holding an entry's spinlock.

Author: Karina Litskevich
Reviewed-by: Michael Paquier, wenhui qiu
Discussion: https://postgr.es/m/CACiT8ibhCmzbcOxM0v4pRLH3abk-95LPkt7_uC2JMP+miPjxsg@mail.gmail.com

diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 1798e1d016fe4695de97e4dd852a4dcea94ec19f..49c657b3e07810ee180d5bd8929e1b9864f9e6e7 100644 (file)
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -1869,9 +1869,14 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
        /* copy counters to a local variable to keep locking time short */
        SpinLockAcquire(&entry->mutex);
        tmp = entry->counters;
+       SpinLockRelease(&entry->mutex);
+
+       /*
+        * The spinlock is not required when reading these two as they are
+        * always updated when holding pgss->lock exclusively.
+        */
        stats_since = entry->stats_since;
        minmax_stats_since = entry->minmax_stats_since;
-       SpinLockRelease(&entry->mutex);
 
        /* Skip entry if unexecuted (ie, it's a pending "sticky" entry) */
        if (IS_STICKY(tmp))