RE: Expose JIT counters/timing in pg_stat_statements

From: "Shinoda, Noriyoshi (PN Japan FSIP)" <noriyoshi(dot)shinoda(at)hpe(dot)com>
To: Magnus Hagander <magnus(at)hagander(dot)net>, Julien Rouhaud <rjuju123(at)gmail(dot)com>
Cc: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: Expose JIT counters/timing in pg_stat_statements
Date: 2022-04-09 01:51:21
Message-ID: PH7PR84MB1885730757AA814C2F0E8CEDEEE89@PH7PR84MB1885.NAMPRD84.PROD.OUTLOOK.COM
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,
thank you for the great features.

The attached small patch changes the data type in the document.
The following columns are actually double precision but bigint in the docs.
jit_generation_time
jit_inlining_time
jit_optimization_time
jit_emission_count

Regards,
Noriyoshi Shinoda

From: Magnus Hagander <magnus(at)hagander(dot)net>
Sent: Friday, April 8, 2022 8:47 PM
To: Julien Rouhaud <rjuju123(at)gmail(dot)com>
Cc: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Expose JIT counters/timing in pg_stat_statements

On Tue, Mar 8, 2022 at 4:08 AM Julien Rouhaud <rjuju123(at)gmail(dot)com<mailto:rjuju123(at)gmail(dot)com>> wrote:
On Mon, Mar 07, 2022 at 01:40:34PM +0100, Magnus Hagander wrote:
>
> I wonder if there might be an interesting middle ground, or if that is
> making it too much. That is, we could have an
> Option 3:
> jit_count
> total_jit_time - for sum of functions+inlining+optimization+emission time
> min_jit_time - for sum of functions+inlining+optimization+emission time
> max_jit_time - for sum of functions+inlining+optimization+emission time
> mean_jit_time - for sum of functions+inlining+optimization+emission time
> stddev_jit_time - for sum of functions+inlining+optimization+emission time
> jit_functions
> jit_generation_time
> jit_inlining_count
> jit_inlining_time
> jit_optimization_count
> jit_optimization_time
> jit_emission_count
> jit_emission_time
>
> That is, we'd get the more detailed timings across the total time, but
> not on the details. But that might be overkill.

I also thought about it but it seems overkill. pg_stat_statements view is
already very big, and I think that the JIT time should be somewhat stable, at
least compared to how much a query execution time can vary depending on the
parameters. This approach would also be a bit useless if you change the
costing of underlying JIT operation.

> But -- here's an updated patched based on Option 2.

Thanks!

Code-wide, the patch looks good. For the doc, it seems that you documented
jit_inlining_count three times rather than documenting jit_optimization_count
and jit_emission_count.

Oops, thanks and fixed.

I don't think we can add tests there, and having a test for every new counter
being >= 0 seems entirely useless, however there should be a new test added for
the "oldextversions" test to make sure that there's no issue with old SQL / new
shlib compatibility. And looking at it I see that it was already missed for
version 1.9 :(

Indeed. Fixed here.

Michael had already applied a patch that took us to 1.10 and added that test, so I've just updated it here. I don't think we normally bump the version twice int he same day, so I just mergd the SQL script changes as well.

PFA a "final" version for the CI to run.

--
Magnus Hagander
Me: https://www.hagander.net/<http://www.hagander.net/>
Work: https://www.redpill-linpro.com/<http://www.redpill-linpro.com/>

Attachment Content-Type Size
pg_stat_statements_jit_doc_v1.diff application/octet-stream 1.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2022-04-09 02:15:03 Re: PostgreSQL commitfest: 2022-09-01
Previous Message Justin Pryzby 2022-04-09 01:10:38 Re: avoid multiple hard links to same WAL file after a crash