Hi Andres,
On Fri, Feb 11, 2022 at 10:29 AM Andres Freund <andres@anarazel.de> wrote:
> On 2022-02-10 17:13:52 +0900, Amit Langote wrote:
> > The attached patch implements this idea. Sorry for the delay in
> > getting this out and thanks to Robert for the off-list discussions on
> > this.
>
> I did not follow this thread at all. And I only skimmed the patch. So I'm
> probably wrong.
Thanks for your interest in this and sorry about the delay in replying
(have been away due to illness).
> I'm a wary of this increasing executor overhead even in cases it won't
> help. Without this patch, for simple queries, I see small allocations
> noticeably in profiles. This adds a bunch more, even if
> !context->stmt->usesPreExecPruning:
Ah, if any new stuff added by the patch runs in
!context->stmt->usesPreExecPruning paths, then it's just poor coding
on my part, which I'm now looking to fix. Maybe not all of it is
avoidable, but I think whatever isn't should be trivial...
> - makeNode(ExecPrepContext)
> - makeNode(ExecPrepOutput)
> - palloc0(sizeof(PlanPrepOutput *) * result->numPlanNodes)
> - stmt_execprep_list = lappend(stmt_execprep_list, execprep);
> - AllocSetContextCreate(CurrentMemoryContext,
> "CachedPlan execprep list", ...
> - ...
>
> That's a lot of extra for something that's already a bottleneck.
If all these allocations are limited to the usesPreExecPruning path,
IMO, they would amount to trivial overhead compared to what is going
to be avoided -- locking say 1000 partitions when only 1 will be
scanned. Although, maybe there's a way to code this to have even less
overhead than what's in the patch now.
--
Amit Langote
EDB: http://www.enterprisedb.com
