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

Hi Team,

As a follow-up to the skip scan regression discussion, I tested a small
patch that introduces *static allocation/caching of `IndexAmRoutine` *objects
in `amapi.c`, removing the malloc/free overhead.

*Test setup :*
- Baseline: PG17 (commit before skip scan)
- After: PG18 build with skip scan (patched)
- pgbench scale=1, 100 partitions
- Query: `select count(*) from pgbench_accounts where bid = 0`
- Clients: 1, 4, 32
- Protocols: simple, prepared

*Results (tps, 10s runs) :*

Mode Clients Before (PG17) After (PG18 w/ static fix)

simple 1 23856 20332 (~15% lower)
simple 4 55299 53184 (~4% lower)
simple 32 79779 78347 (~2% lower)

prepared 1 26364 26615 (no regression)
prepared 4 55784 54437 (~2% lower)
prepared 32 84687 80374 (~5% lower)

This shows the static fix eliminates the severe ~50% regression previously
observed by Tomas, leaving only a small residual slowdown (*~2-15%*).

*Patch summary :*
- Cache `IndexAmRoutine` instances per AM OID instead of malloc/free per
call.
- Avoid `pfree(amroutine)` in hot paths.
- Keeps allocations stable across lookups, reducing malloc churn.

*Proposal :*
I suggest adopting this static allocation approach for PG18 to prevent
performance cliffs. Longer term, we can explore lighter-weight caching
mechanisms or further executor tuning.

*Patch attached for discussion.*

Thanks & Regards,
Athiyaman M

On Sat, Aug 30, 2025 at 4:37 AM Tomas Vondra <tomas(at)vondra(dot)me> wrote:

> On 8/29/25 21:03, Peter Geoghegan wrote:
> > On Fri, Aug 29, 2025 at 9:10 AM Tomas Vondra <tomas(at)vondra(dot)me> wrote:
> >> Peter, any thoughts on this. Do you think it's reasonable / feasible to
> >> push the fix?
> >
> > I don't feel comfortable pushing that fix today.
> >
>
> Understood.
>
> > Honestly, I'm still not sure what to do. My proposal was to just
> > remove the totally unused options support function, which is probably
> > fine. But since I don't really know why Alexander ever added the
> > "options" support function in the first place (I don't even see a
> > theoretical benefit), I'm not quite prepared to say that I know that
> > it's okay to remove it now.
> >
>
> Right. I think removing the "options" is the only feasible solution for
> PG18 at this point. Either that or nothing. The other patch is far too
> invasive.
>
> As for why the support procedure was added to existing index AMs, I
> don't know. I suppose it as mostly for consistency, so that custom
> oclasses could opclasses could use that. I have no idea if there are
> plausible custom opclasses using this.
>
> I'm not sure how I feel about removing the support proc. It feels pretty
> arbitrary and fragile, and IIRC it doesn't even address the perf issue
> (add a couple partitions and it'll hit the same issue). It just restores
> the "threshold" to where it was for PG17. And it's fragile, because we
> have no protections about hitting this glibc-specific behavior again. It
> takes one new flag added somewhere, and we'll not even notice it.
>
> So after thinking about this a bit more, and refreshing the context, I
> think the right solution for PG18 is to do nothing.
>
> regards
>
> --
> Tomas Vondra
>
>
>
>