From: | Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Andreas Joseph Krogh <andreas(at)visena(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Query is over 2x slower with jit=on |
Date: | 2018-09-26 18:15:35 |
Message-ID: | CAJ3gD9drS7hJpfZ-S_bAjq1wC=VQ5y53+oWx=9TX7B6GcprV2Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, 25 Sep 2018 at 14:17, Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> On 2018-09-18 10:03:02 +0530, Amit Khandekar wrote:
> > Attached v3 patch that does the above change.
>
> Attached is a revised version of that patch. I've changed quite a few
> things:
> - I've reverted the split of "base" and "provider specific" contexts - I
> don't think it really buys us anything here.
The idea was to have a single estate field that accumulates all the
JIT counters of leader as well as workers. I see that you want to
delay the merging of workers and backend counters until end of query
execution. More points on this in the bottom section.
>
> - I've reverted the context creation changes - instead of creating a
> context in the leader just to store instrumentation in the worker,
> there's now a new EState->es_jit_combined_instr.
The context created in the leader was a light context, involving only
the resource owner stuff, and not the provider initialization.
>
> - That also means worker instrumentation doesn't get folded into the
> leader's instrumentation.
You mean the worker instrumentation doesn't get folded into the leader
until the query execution end, right ? In the committed code, I see
that now we merge the leader instrumentation into the combined worker
instrumentation in standard_ExecutorEnd().
> This seems good for the future and for
> extensions - it's not actually "linear" time that's spent doing
> JIT in workers (& leader), as all of that work happens in
> parallel. Being able to disentangle that seems important.
Ok. Your point is: we should have the backend and workers info stored
in two separate fields, and combine them only when we need it; so that
we will be in a position to show combined workers-only info separately
in the future. From the code, it looks like the es_jit_combined_instr
stores combined workers info not just from a single Gather node, but
all the Gather nodes in the plan. If we want to have separate workers
info, I am not sure if it makes sense in combining workers from two
separate Gather nodes; because these two sets of workers are
unrelated, aren't they ?
>
> This needs a bit more polish tomorrow, but I'm starting to like where
> this is going. Comments?
Yeah, I think the plan output looks reasonable compact now. Thanks.
--
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2018-09-26 18:28:21 | Re: Query is over 2x slower with jit=on |
Previous Message | Andres Freund | 2018-09-26 17:46:45 | Re: Allowing printf("%m") only where it actually works |