Re: Query is over 2x slower with jit=on

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-11 09:20:41
Message-ID: CAJ3gD9fJcYofmWCe9hJ9Y6dUYU-qpkR=Bii+NhYufE7Mny9AHw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 10 September 2018 at 21:39, Andres Freund <andres(at)anarazel(dot)de> wrote:
> Hi,
>
> On 2018-09-10 15:42:55 +0530, Amit Khandekar wrote:
>> Attached is a patch that accumulates the per-worker jit counters into
>> the leader process.
>
> Thanks!
>
>
>> I think we better show per-worker jit info also. The current patch
>> does not show that. I think it would be easy to continue on the patch
>> to show per-worker info also. Under the Gather node, we can show
>> per-worker jit counters. I think this would be useful too, besides the
>> cumulative figures in the leader process. Comments ?
>
> Yes, I think that'd be good.
Ok. Will continue on the patch.

> I think we either should print the stats at
> the top level as $leader_value, $combined_worker_value, $total_value or
> just have the $combined_worker_value at the level where we print other
> stats from the worker, too.

Yes, I think we can follow and be consistent with whatever way in
which the other worker stats are printed. Will check.

Note: Since there can be a multiple separate Gather plans under a plan
tree, I think we can show this info for each Gather plan.

>
>
>> /*
>> + * Add up the workers' JIT instrumentation from dynamic shared memory.
>> + */
>> +static void
>> +ExecParallelRetrieveJitInstrumentation(PlanState *planstate,
>> + SharedJitInstrumentation *shared_jit)
>> +{
>> + int n;
>> + JitContext *jit = planstate->state->es_jit;
>> +
>> + /* If the leader hasn't yet created a jit context, allocate one now. */
>> + if (!jit)
>> + {
>> + planstate->state->es_jit = jit =
>> + jit_create_context(planstate->state->es_jit_flags);
>> + }
>
> Wouldn't it be better to move the jit instrumentation to outside of the
> context, to avoid having to do this? Or just cope with not having
> instrumentation for the leader in this case? We'd kinda need to deal
> with failure to create one anyway?

Yeah, I think taking out the instrumentation out of the context looks
better. Will work on that.

--
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2018-09-11 09:54:16 Re: StandbyAcquireAccessExclusiveLock doesn't necessarily
Previous Message Michael Paquier 2018-09-11 09:00:02 Re: libpq stricter integer parsing