On 5/11/25 18:07, Peter Geoghegan wrote:
> On Sat, May 10, 2025 at 10:59 AM Tomas Vondra <to...@vondra.me> wrote:
>> But doesn't it also highlight how fragile this memory allocation is? The
>> skip scan patch didn't do anything wrong - it just added a couple
>> fields, using a little bit more memory. I think we understand allocating
>> more memory may need more time, but we expect the effect to be somewhat
>> proportional. Which doesn't seem to be the case here.
>>
>> Many other patches add fields somewhere, it seems like bad luck the skip
>> scan happened to trigger this behavior. It's quite likely other patches
>> ran into the same issue, except that no one noticed. Maybe the skip scan
>> did that in much hotter code, not sure.
>
> But what did the skip scan commit (specifically commit 92fe23d9,
> without any of the follow-up commits) change about memory allocation,
> that might be at issue with your test case? You said that that commit
> "just added a couple fields". What specific fields are you talking
> about, that were added by commit 92fe23d9?
>
> I already speculated that the issue might be tied to the addition of a
> new support routine (skip support), but the experiment we ran to try
> to validate that theory disproved it. What else is there?
>
That's a good point. However, it seems I have done something wrong when
running the tests with the support routine removed :-( I just repeated
the tests, and I got this:
mode clients 3ba2cdaa454 master revert master revert
------------------------------------------------------------------
prepared 1 10860 3548 11048 33% 102%
4 25492 11299 25190 44% 99%
32 38399 14142 38493 37% 100%
------------------------------------------------------------------
simple 1 2595 1844 2604 71% 100%
4 8266 6090 8126 74% 98%
32 11765 7198 11449 61% 97%
I where "revert" is master with the removal patch. Sorry about the
confusion, I guess I was distracted and did some mistake.
So, this seems to be in line with the hypothesis ...
regards
--
Tomas Vondra
