From: | "Drouvot, Bertrand" <bertranddrouvot(dot)pg(at)gmail(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Avoid double lookup in pgstat_fetch_stat_tabentry() |
Date: | 2022-11-19 08:38:26 |
Message-ID: | 5b82cad3-218d-eb02-61aa-11723a1083a9@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
v3-0001-pgstat_fetch_stat_tabentry-avoid_double_lookup.patch | text/plain | 969 bytes |
From | Date | Subject | |
---|---|---|---|
Next Message | Tomas Vondra | 2022-11-19 10:34:07 | Re: test/modules/test_oat_hooks vs. debug_discard_caches=1 |
Previous Message | Gilles Darold | 2022-11-19 06:30:59 | Re: [BUG] pg_dump blocked |