jit and explain nontext

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

jit and explain nontext

Justin Pryzby
        /* don't print information if no JITing happened */
        if (!ji || ji->created_functions == 0)
                return;

This applies even when (es->format != EXPLAIN_FORMAT_TEXT), which I think is
wrong.  Jit use can be determined by cost, so I think jit details should be
shown in non-text format whenever ji!=NULL, even if it's zeros.  Arguably, bits
could be omitted if jit_expressions=off or jit_tuple_deforming=off, but I don't
see the point.

--
Justin


Reply | Threaded
Open this post in threaded view
|

Re: jit and explain nontext

David Rowley
On Thu, 15 Oct 2020 at 08:39, Justin Pryzby <[hidden email]> wrote:

>
>         /* don't print information if no JITing happened */
>         if (!ji || ji->created_functions == 0)
>                 return;
>
> This applies even when (es->format != EXPLAIN_FORMAT_TEXT), which I think is
> wrong.  Jit use can be determined by cost, so I think jit details should be
> shown in non-text format whenever ji!=NULL, even if it's zeros.  Arguably, bits
> could be omitted if jit_expressions=off or jit_tuple_deforming=off, but I don't
> see the point.

Just for some reference. Some wisdom was shared in [1], which made a
lot of sense to me.

If we apply that, then we just need to decide if displaying any jit
related fields without any jitted expressions is relevant.

I'm a bit undecided.

[1] https://www.postgresql.org/message-id/2276865.1593102811%40sss.pgh.pa.us


Reply | Threaded
Open this post in threaded view
|

Re: jit and explain nontext

Tom Lane-2
David Rowley <[hidden email]> writes:
> Just for some reference. Some wisdom was shared in [1], which made a
> lot of sense to me.
> If we apply that, then we just need to decide if displaying any jit
> related fields without any jitted expressions is relevant.

Hmm, I dunno if my opinion counts as "wisdom", but what I was arguing for
there was that we should print stuff if it's potentially invoked by a
run-time decision, but not if it was excluded at plan time.  I'm not
totally clear on whether jitting decisions are fixed by the plan tree
(including its cost values) or if the executor can make different
decisions in different executions of the identical plan tree.
If the latter, then I agree with Justin that this is a bug.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: jit and explain nontext

David Rowley
On Thu, 15 Oct 2020 at 14:15, Tom Lane <[hidden email]> wrote:

>
> David Rowley <[hidden email]> writes:
> > Just for some reference. Some wisdom was shared in [1], which made a
> > lot of sense to me.
> > If we apply that, then we just need to decide if displaying any jit
> > related fields without any jitted expressions is relevant.
>
> Hmm, I dunno if my opinion counts as "wisdom", but what I was arguing for
> there was that we should print stuff if it's potentially invoked by a
> run-time decision, but not if it was excluded at plan time.  I'm not
> totally clear on whether jitting decisions are fixed by the plan tree
> (including its cost values) or if the executor can make different
> decisions in different executions of the identical plan tree.
> If the latter, then I agree with Justin that this is a bug.

As far as I know, the only exception where the executor overwrites the
planner's decision is in nodeValuesscan.c where it turns jit off
because each VALUES will get evaluated just once, which would be a
waste of effort to JIT.

Apart from that the choice is baked in by the planner and set in
PlannedStmt.jitfFlags.

David


Reply | Threaded
Open this post in threaded view
|

Re: jit and explain nontext

Justin Pryzby
On Thu, Oct 15, 2020 at 02:23:01PM +1300, David Rowley wrote:

> On Thu, 15 Oct 2020 at 14:15, Tom Lane <[hidden email]> wrote:
> > David Rowley <[hidden email]> writes:
> > > Just for some reference. Some wisdom was shared in [1], which made a
> > > lot of sense to me.
> > > If we apply that, then we just need to decide if displaying any jit
> > > related fields without any jitted expressions is relevant.
> >
> > Hmm, I dunno if my opinion counts as "wisdom", but what I was arguing for
> > there was that we should print stuff if it's potentially invoked by a
> > run-time decision, but not if it was excluded at plan time.  I'm not
> > totally clear on whether jitting decisions are fixed by the plan tree
> > (including its cost values) or if the executor can make different
> > decisions in different executions of the identical plan tree.
> > If the latter, then I agree with Justin that this is a bug.
>
> As far as I know, the only exception where the executor overwrites the
> planner's decision is in nodeValuesscan.c where it turns jit off
> because each VALUES will get evaluated just once, which would be a
> waste of effort to JIT.
>
> Apart from that the choice is baked in by the planner and set in
> PlannedStmt.jitfFlags.

What about the GUCs themselves ?

They can change after planning, which means a given execution of a plan might
or might not use jit.

--
Justin


Reply | Threaded
Open this post in threaded view
|

Re: jit and explain nontext

David Rowley
On Thu, 15 Oct 2020 at 14:43, Justin Pryzby <[hidden email]> wrote:

>
> On Thu, Oct 15, 2020 at 02:23:01PM +1300, David Rowley wrote:
> > On Thu, 15 Oct 2020 at 14:15, Tom Lane <[hidden email]> wrote:
> > > Hmm, I dunno if my opinion counts as "wisdom", but what I was arguing for
> > > there was that we should print stuff if it's potentially invoked by a
> > > run-time decision, but not if it was excluded at plan time.  I'm not
> > > totally clear on whether jitting decisions are fixed by the plan tree
> > > (including its cost values) or if the executor can make different
> > > decisions in different executions of the identical plan tree.
> > > If the latter, then I agree with Justin that this is a bug.
> >
> > As far as I know, the only exception where the executor overwrites the
> > planner's decision is in nodeValuesscan.c where it turns jit off
> > because each VALUES will get evaluated just once, which would be a
> > waste of effort to JIT.
> >
> > Apart from that the choice is baked in by the planner and set in
> > PlannedStmt.jitfFlags.
>
> What about the GUCs themselves ?
>
> They can change after planning, which means a given execution of a plan might
> or might not use jit.

That's a pretty good point.  If we do SET enable_sort TO off; then
cached plans are unaffected.  That's not the case when someone does
SET jit TO off; as we'll check that in provider_init() during
execution.  Although, switching jit back on again works differently.
If the planner saw it was off then switching it on again won't have
existing plans use it.  That's slightly weird, but perhaps it was done
that way to ensure there was a hard off switch. You might want to
ensure that to ensure queries don't break if there was some problem
with LLVM libraries.

David


Reply | Threaded
Open this post in threaded view
|

Re: jit and explain nontext

Andres Freund
Hi,

On 2020-10-15 14:51:38 +1300, David Rowley wrote:
> That's a pretty good point.  If we do SET enable_sort TO off; then
> cached plans are unaffected.

In contrast to that we do however use the current work_mem for
execution, I believe. Which could be fairly nasty, if a plan was made
for a huge work_mem, for example.


> That's not the case when someone does SET jit TO off; as we'll check
> that in provider_init() during execution.  Although, switching jit
> back on again works differently.  If the planner saw it was off then
> switching it on again won't have existing plans use it.  That's
> slightly weird, but perhaps it was done that way to ensure there was a
> hard off switch.

It was motivated by not wanting to just enable JIT for queries that were
prepared within something like SET LOCAL jit=off;PREPARE; RESET
jit;. I'm open to revising it, but that's where it's coming from.

Greetings,

Andres Freund


Reply | Threaded
Open this post in threaded view
|

Re: jit and explain nontext

Justin Pryzby
In reply to this post by David Rowley
On Thu, Oct 15, 2020 at 02:51:38PM +1300, David Rowley wrote:

> On Thu, 15 Oct 2020 at 14:43, Justin Pryzby <[hidden email]> wrote:
> > On Thu, Oct 15, 2020 at 02:23:01PM +1300, David Rowley wrote:
> > > On Thu, 15 Oct 2020 at 14:15, Tom Lane <[hidden email]> wrote:
> > > > Hmm, I dunno if my opinion counts as "wisdom", but what I was arguing for
> > > > there was that we should print stuff if it's potentially invoked by a
> > > > run-time decision, but not if it was excluded at plan time.  I'm not
> > > > totally clear on whether jitting decisions are fixed by the plan tree
> > > > (including its cost values) or if the executor can make different
> > > > decisions in different executions of the identical plan tree.
> > > > If the latter, then I agree with Justin that this is a bug.
> > >
> > > As far as I know, the only exception where the executor overwrites the
> > > planner's decision is in nodeValuesscan.c where it turns jit off
> > > because each VALUES will get evaluated just once, which would be a
> > > waste of effort to JIT.
> > >
> > > Apart from that the choice is baked in by the planner and set in
> > > PlannedStmt.jitfFlags.
> >
> > What about the GUCs themselves ?
> >
> > They can change after planning, which means a given execution of a plan might
> > or might not use jit.
>
> That's a pretty good point.
Added at: https://commitfest.postgresql.org/30/2766/

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 41317f1837..7345971507 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -839,7 +839,8 @@ ExplainPrintJIT(ExplainState *es, int jit_flags, JitInstrumentation *ji)
  instr_time total_time;
 
  /* don't print information if no JITing happened */
- if (!ji || ji->created_functions == 0)
+ if (!ji || (ji->created_functions == 0 &&
+ es->format == EXPLAIN_FORMAT_TEXT))
  return;
 
  /* calculate total time */
--
2.17.0

v1-0001-explain-show-JIT-details-in-non-text-format-even-.patch (892 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: jit and explain nontext

David Rowley
On Sun, 18 Oct 2020 at 08:21, Justin Pryzby <[hidden email]> wrote:
>         /* don't print information if no JITing happened */
> -       if (!ji || ji->created_functions == 0)
> +       if (!ji || (ji->created_functions == 0 &&
> +                       es->format == EXPLAIN_FORMAT_TEXT))
>                 return;

Isn't that comment now outdated?

I imagine something more like; /* Only show JIT details when we jitted
something or when in non-text mode */ might be better after making
that code change.

David