Re: Adding skip scan (including MDAM style range skip scan) to nbtree

On 5/11/25 18:07, Peter Geoghegan wrote:
> On Sat, May 10, 2025 at 10:59 AM Tomas Vondra <tomas@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