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
