auto_explain: Include JIT output if applicable

Started by Lukas Fittlover 7 years ago6 messageshackers
Jump to latest
#1Lukas Fittl
lukas@fittl.com

Hi,

Whilst playing around with auto_explain and JIT today, I realized that
auto_explain currently doesn't output JIT information, which is rather
unfortunate when analyzing a larger set of queries in a semi-automated
manner.

Attached a trivial patch that fixes the issue and adds JIT information to
auto_explain with the same logic as used for regular EXPLAIN.

Thanks,
Lukas

--
Lukas Fittl

Attachments:

auto_explain-include-jit-output-v1.patchapplication/octet-stream; name=auto_explain-include-jit-output-v1.patchDownload+4-1
#2Andres Freund
andres@anarazel.de
In reply to: Lukas Fittl (#1)
Re: auto_explain: Include JIT output if applicable

Hi,

On 2018-09-24 11:34:38 -0700, Lukas Fittl wrote:

Hi,

Whilst playing around with auto_explain and JIT today, I realized that
auto_explain currently doesn't output JIT information, which is rather
unfortunate when analyzing a larger set of queries in a semi-automated
manner.

Attached a trivial patch that fixes the issue and adds JIT information to
auto_explain with the same logic as used for regular EXPLAIN.

Thanks for noticing - pushed!

It's pretty annoying that so much of this code is duplicated in
auto_explain. It'd be good if we refactored explain.c so that there's
less duplication. But that seems like it'd not be v11 work, so...

- Andres

#3Lukas Fittl
lukas@fittl.com
In reply to: Andres Freund (#2)
Re: auto_explain: Include JIT output if applicable

On Mon, Sep 24, 2018 at 1:48 PM, Andres Freund <andres@anarazel.de> wrote:

Thanks for noticing - pushed!

Thanks!

Best,
Lukas

--
Lukas Fittl

#4Shinoda, Noriyoshi (PN Japan FSIP)
noriyoshi.shinoda@hpe.com
In reply to: Lukas Fittl (#3)
RE: auto_explain: Include JIT output if applicable

Hi,

I tried this feature.
I think that 'if (es->costs)' of the source code auto_explain.c will always be ‘true’.

Because it is not changed after 'es-> cost = true' in NewExplainState () function several rows ago.
So I attached a patch to delete this if statement.

Regards,
Noriyoshi Shinoda

From: Lukas Fittl [mailto:lukas@fittl.com]
Sent: Tuesday, September 25, 2018 6:38 AM
To: Andres Freund <andres@anarazel.de>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: auto_explain: Include JIT output if applicable

On Mon, Sep 24, 2018 at 1:48 PM, Andres Freund <andres@anarazel.de<mailto:andres@anarazel.de>> wrote:
Thanks for noticing - pushed!

Thanks!

Best,
Lukas

--
Lukas Fittl

Attachments:

auto_explain.diffapplication/octet-stream; name=auto_explain.diffDownload+3-3
#5Andres Freund
andres@anarazel.de
In reply to: Shinoda, Noriyoshi (PN Japan FSIP) (#4)
Re: auto_explain: Include JIT output if applicable

Hi,

On 2018-09-29 05:04:25 +0000, Shinoda, Noriyoshi (PN Japan GCS Delivery) wrote:

I tried this feature.
I think that 'if (es->costs)' of the source code auto_explain.c will always be ‘true’.

Because it is not changed after 'es-> cost = true' in NewExplainState () function several rows ago.
So I attached a patch to delete this if statement.

I think it's better to stay closer to what explain.c itself is doing -
it's not like that if statement costs us anything really...

- Andres

#6Shinoda, Noriyoshi (PN Japan FSIP)
noriyoshi.shinoda@hpe.com
In reply to: Andres Freund (#5)
RE: auto_explain: Include JIT output if applicable

Hi,

I think it's better to stay closer to what explain.c itself is doing - it's not like that if statement costs us anything really...

Oh, I understood.
Thank you.

-----Original Message-----
From: Andres Freund [mailto:andres@anarazel.de]
Sent: Saturday, September 29, 2018 2:11 PM
To: Shinoda, Noriyoshi (PN Japan GCS Delivery) <noriyoshi.shinoda@hpe.com>
Cc: Lukas Fittl <lukas@fittl.com>; Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: auto_explain: Include JIT output if applicable

Hi,

On 2018-09-29 05:04:25 +0000, Shinoda, Noriyoshi (PN Japan GCS Delivery) wrote:

I tried this feature.
I think that 'if (es->costs)' of the source code auto_explain.c will always be ‘true’.

Because it is not changed after 'es-> cost = true' in NewExplainState () function several rows ago.
So I attached a patch to delete this if statement.

I think it's better to stay closer to what explain.c itself is doing - it's not like that if statement costs us anything really...

- Andres