Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2019-Aug-09, Tom Lane wrote:
>> I poked at this, and attached is a patch, but again I'm not seeing
>> that there's any real performance-based argument for it.
> I'm confused. I thought that the point of doing this wasn't that we
> wanted to improve performance, but rather that we're now able to remove
> the array without *losing* performance. I mean, those arrays were there
> to improve performance for code that wanted fast access to specific list
> items, but now we have fast access to the list items without it. So a
> measurement that finds no performance difference is good news, and we
> can get rid of the now-pointless optimization ...
Yeah, fair enough, so pushed.
In principle, this change adds an extra indirection in exec_rt_fetch,
so I went looking to see if there were any such calls in arguably
performance-critical paths. Unsurprisingly, most calls are in executor
initialization, and they tend to be adjacent to table_open() or other
expensive operations, so it's pretty hard to claim that there could
be any measurable hit. However, I did notice that trigger.c uses
ExecUpdateLockMode() and GetAllUpdatedColumns() in ExecBRUpdateTriggers
which executes per-row, and so might be worth trying to optimize.
exec_rt_fetch itself is not the main cost in either of those, but I wonder
why we are doing those calculations over again for each row in the first
place. I'm not excited enough about the issue to do anything right now,
but the next time somebody whines about trigger-firing overhead, there
might be an easy win available there.
regards, tom lane
