Hi,
On 11/18/22 6:32 PM, Andres Freund wrote:
Hi,
On 2022-11-18 11:09:43 +0100, Drouvot, Bertrand wrote:
Furthermore, the pgstat_fetch_stat_tabentry() can just be a
static inline function.
I think that's just premature optimization for something like this. The
function call overhead on accessing stats can't be a relevant factor - the
increase in code size is more likely to matter (but still unlikely).
Good point. While at it, why not completely get rid of
pgstat_fetch_stat_tabentry_ext(), like in v2 the attached?
-1, I don't think spreading the IsSharedRelation() is a good idea. It costs
more code than it saves.
Got it, please find attached V3: switching back to the initial proposal and
implementing Bharath's comment (getting rid of the local variable tabentry).
Out of curiosity, here are the sizes (no debug):
- Current code (no patch)
$ size ./src/backend/utils/adt/pgstatfuncs.o
./src/backend/utils/activity/pgstat_relation.o
text data bss dec hex filename
24974 0 0 24974 618e ./src/backend/utils/adt/pgstatfuncs.o
7353 64 0 7417 1cf9
./src/backend/utils/activity/pgstat_relation.o
- IsSharedRelation() spreading
$ size ./src/backend/utils/adt/pgstatfuncs.o
./src/backend/utils/activity/pgstat_relation.o
text data bss dec hex filename
25304 0 0 25304 62d8 ./src/backend/utils/adt/pgstatfuncs.o
7249 64 0 7313 1c91
./src/backend/utils/activity/pgstat_relation.o
- inline function
$ size ./src/backend/utils/adt/pgstatfuncs.o
./src/backend/utils/activity/pgstat_relation.o
text data bss dec hex filename
25044 0 0 25044 61d4 ./src/backend/utils/adt/pgstatfuncs.o
7249 64 0 7313 1c91
./src/backend/utils/activity/pgstat_relation.o
- V3 attached
$ size ./src/backend/utils/adt/pgstatfuncs.o
./src/backend/utils/activity/pgstat_relation.o
text data bss dec hex filename
24974 0 0 24974 618e ./src/backend/utils/adt/pgstatfuncs.o
7323 64 0 7387 1cdb
./src/backend/utils/activity/pgstat_relation.o
I'd vote for V3 for readability, size and "backward compatibility" with current
code.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
diff --git a/src/backend/utils/activity/pgstat_relation.c
b/src/backend/utils/activity/pgstat_relation.c
index 55a355f583..f92e16e7af 100644
--- a/src/backend/utils/activity/pgstat_relation.c
+++ b/src/backend/utils/activity/pgstat_relation.c
@@ -25,6 +25,7 @@
#include "utils/pgstat_internal.h"
#include "utils/rel.h"
#include "utils/timestamp.h"
+#include "catalog/catalog.h"
/* Record that's written to 2PC state file when pgstat state is persisted */
@@ -437,17 +438,7 @@ pgstat_update_heap_dead_tuples(Relation rel, int delta)
PgStat_StatTabEntry *
pgstat_fetch_stat_tabentry(Oid relid)
{
- PgStat_StatTabEntry *tabentry;
-
- tabentry = pgstat_fetch_stat_tabentry_ext(false, relid);
- if (tabentry != NULL)
- return tabentry;
-
- /*
- * If we didn't find it, maybe it's a shared table.
- */
- tabentry = pgstat_fetch_stat_tabentry_ext(true, relid);
- return tabentry;
+ return pgstat_fetch_stat_tabentry_ext(IsSharedRelation(relid), relid);
}
/*
