Re: Avoid double lookup in pgstat_fetch_stat_tabentry() - Mailing list pgsql-hackers

From Drouvot, Bertrand
Subject Re: Avoid double lookup in pgstat_fetch_stat_tabentry()
Date
Msg-id 5b82cad3-218d-eb02-61aa-11723a1083a9@gmail.com
Whole thread Raw
In response to Re: Avoid double lookup in pgstat_fetch_stat_tabentry()  (Andres Freund <andres@anarazel.de>)
Responses Re: Avoid double lookup in pgstat_fetch_stat_tabentry()
List 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
ofthe 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

pgsql-hackers by date:

Previous
From: Gilles Darold
Date:
Subject: Re: [BUG] pg_dump blocked
Next
From: Tomas Vondra
Date:
Subject: Re: test/modules/test_oat_hooks vs. debug_discard_caches=1