Re: Use get_call_result_type() more widely - Mailing list pgsql-hackers

From Bharath Rupireddy
Subject Re: Use get_call_result_type() more widely
Date
Msg-id CALj2ACUvLxfhMcve6wCKG+05Z61sC3RTLDmAGYReOoSs4pZdQg@mail.gmail.com
Whole thread Raw
In response to Re: Use get_call_result_type() more widely  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Use get_call_result_type() more widely  (Michael Paquier <michael@paquier.xyz>)
List pgsql-hackers
On Tue, Dec 13, 2022 at 9:12 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:
> > A review comment in another thread [1] by Michael Paquier about the
> > usage of get_call_result_type() instead of explicit building of
> > TupleDesc made me think about using it more widely. Actually, the
> > get_call_result_type() looks at the function definitions to figure the
> > column names and build the required TupleDesc, usage of which avoids
> > duplication of the column names between pg_proc.dat/function
> > definitions and source code. Also, it saves a good number of LOC ~415
> > [2] and the size of all the object files put together gets reduced by
> > ~4MB, which means, the postgres binary becomes leaner by ~4MB [3].
>
> Saving code is nice, but I'd assume the result is slower, because
> get_call_result_type has to do a pretty substantial amount of work
> to get the data to construct the tupdesc from.  Have you tried to
> quantify how much overhead this'd add?  Which of these functions
> can we safely consider to be non-performance-critical?

AFAICS, most of these functions have no direct source code callers,
they're user-facing functions and not in a hot code path. I measured
the test times of these functions and I don't see much difference [1].

[1]
pg_old_snapshot_time_mapping() - an extension function with no
internal source code callers, no test coverage.
pg_visibility_map_summary() - an extension function with no internal
source code callers, test coverage exists, test times on HEAD:25 ms
PATCHED:25 ms
pg_last_committed_xact() and pg_xact_commit_timestamp_origin() - no
internal source code callers, test coverage exists, test times on
HEAD:10 ms PATCHED:10 ms
pg_get_multixact_members() - no internal source code callers, no test coverage.
pg_prepared_xact() - no internal source code callers, test coverage
exists, test times on HEAD:50 ms, subscription 108 wallclock secs,
recovery 111 wallclock secs PATCHED:48 ms, subscription 110 wallclock
secs, recovery 112 wallclock secs
pg_walfile_name_offset() - no internal source code callers, no test coverage.
pg_get_object_address() - no internal source code callers, test
coverage exists, test times on HEAD:66 ms PATCHED:60 ms
pg_identify_object() - no internal source code callers, test coverage
exists, test times on HEAD:17 ms PATCHED:18 ms
pg_identify_object_as_address() - no internal source code callers,
test coverage exists, test times on HEAD:66 ms PATCHED:60 ms
pg_get_publication_tables() - internal source code callers exist, test
coverage exists, test times on HEAD:159 ms, subscription 108 wallclock
secs PATCHED:167 ms, subscription 110 wallclock secs
pg_sequence_parameters() - no internal source code callers, test
coverage exists, test times on HEAD:96 ms PATCHED:98 ms
ts_token_type_byid(), ts_token_type_byname(), ts_parse_byid() and
ts_parse_byname() - internal source code callers exists, test coverage
exists, test times on HEAD:195 ms, pg_dump 10 wallclock secs
PATCHED:186 ms, pg_dump 10 wallclock secs
aclexplode() - internal callers exists information_schema.sql,
indirect test coverage exists.
pg_timezone_abbrevs() - no internal source code callers, test coverage
exists, test times on HEAD:40 ms PATCHED:36 ms
pg_stat_file() - no internal source code callers, test coverage
exists, test times on HEAD:42 ms PATCHED:46 ms
pg_lock_status() - no internal source code callers, test coverage
exists, test times on HEAD:16 ms PATCHED:22 ms
pg_get_keywords() - no internal source code callers, test coverage
exists, test times on HEAD:129 ms PATCHED:130 ms
pg_get_catalog_foreign_keys() - no internal source code callers, test
coverage exists, test times on HEAD:114 ms PATCHED:111 ms
pg_partition_tree() - no internal source code callers, test coverage
exists, test times on HEAD:30 ms PATCHED:32 ms
pg_stat_get_wal(), pg_stat_get_archiver() and
pg_stat_get_replication_slot() - no internal source code callers, test
coverage exists, test times on HEAD:479 ms PATCHED:483 ms
pg_stat_get_subscription_stats() - no internal source code callers,
test coverage exists, test times on HEAD:subscription 108 wallclock
secs PATCHED:subscription 110 wallclock secs
tsvector_unnest() - no internal source code callers, test coverage
exists, test times on HEAD:26 ms PATCHED:26 ms
ts_setup_firstcall() - test coverage exists, test times on HEAD:195 ms
PATCHED:186 ms
show_all_settings(), pg_control_system(), pg_control_checkpoint(),
pg_control_recovery() and pg_control_init() - test coverage exists,
test times on HEAD:42 ms PATCHED:44 ms
test_predtest() - no internal source code callers, test coverage
exists, test times on HEAD:18 ms PATCHED:18 ms

-- 
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com



pgsql-hackers by date:

Previous
From: Erik Rijkers
Date:
Subject: Re: Schema variables - new implementation for Postgres 15 (typo)
Next
From: Kyotaro Horiguchi
Date:
Subject: Re: Shortening the Scope of Critical Section in Creation of New MultiXacts