On 23/10/2020 12:37, Amit Langote wrote:
> To explain these numbers a bit, "overheaul update/delete processing"
> patch improves the performance of that benchmark by allowing the
> updates to use run-time pruning when executing generic plans, which
> they can't today.
>
> However without "lazy-ResultRelInfo-initialization" patch,
> ExecInitModifyTable() (or InitPlan() when I ran those benchmarks) can
> be seen to be spending time initializing all of those result
> relations, whereas only one of those will actually be used.
>
> As mentioned further in that email, it's really the locking of all
> relations by AcquireExecutorLocks() that occurs even before we enter
> the executor that's a much thornier bottleneck for this benchmark.
> But the ResultRelInfo initialization bottleneck sounded like it could
> get alleviated in a relatively straightforward manner. The patches
> that were developed for attacking the locking bottleneck would require
> further reflection on whether they are correct.
>
> (Note: I've just copy pasted the numbers I reported in that email. To
> reproduce, I'll have to rebase the "overhaul update/delete processing"
> patch on this one, which I haven't yet done.)
Ok, thanks for the explanation, now I understand.
This patch looks reasonable to me at a quick glance. I'm a bit worried
or unhappy about the impact on FDWs, though. It doesn't seem nice that
the ResultRelInfo is not available in the BeginDirectModify call. It's
not too bad, the FDW can call ExecGetResultRelation() if it needs it,
but still. Perhaps it would be better to delay calling
BeginDirectModify() until the first modification is performed, to avoid
any initialization overhead there, like establishing the connection in
postgres_fdw.
But since this applies on top of the "overhaul update/delete processing"
patch, let's tackle that patch set next. Could you rebase that, please?
- Heikki