On 10 September 2018 at 21:39, Andres Freund <andres@anarazel.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
