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 8/29/25 21:03, Peter Geoghegan wrote:
> On Fri, Aug 29, 2025 at 9:10 AM Tomas Vondra <tomas@vondra.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
