| From: | Andres Freund <andres(at)anarazel(dot)de> |
|---|---|
| To: | Haibo Yan <tristan(dot)yim(at)gmail(dot)com> |
| Cc: | Matthias Dötsch <matze(at)mdoetsch(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org, amitlan(at)postgresql(dot)org, dgrowleyml(at)gmail(dot)com |
| Subject: | Re: Perf: Specialize ExecIndexOnlyScan() like ExecSeqScan() |
| Date: | 2026-10-14 19:14:10 |
| Message-ID: | 6o556zb732u6fogc2lanfpideyhqmfuthxvslbd4orelyjvng5@oetgfpugjn3d |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On 2026-10-14 11:12:57 -0700, Haibo Yan wrote:
> However, I wonder whether we should evaluate the broader tradeoffs
> before introducing more hand-specialized executor variants. While
> the optimization makes sense in isolation, I'm less certain about
> its long-term benefits relative to the additional code complexity
> and maintenance cost.
>
> In particular, it would be interesting to see how much of the
> improvement survives with LTO/PGO enabled. Both can help with
> inlining and code layout, potentially reducing the benefit of
> manual specialization.
I really don't see how it could obsolete the improvement in the patch (it
could help at the same time obviously, but the patch should improve LTO and
non-LTO). The compiler has no way of knowing that estate->es_epq_active,
indexstate->ss.ps.qual, indexstate->ss.ps.ps_ProjInfo don't change during
query execution, that's something only we know.
It's possible that that some LTO implementations would end up inlining
ExecSeqScan() and then convert the indirect function call to IndexOnlyNext()
into a [potentially guarded] direct function call. But that's only a
relatively small portion of the benefit of the specializations!
There's is *some* support for speculative assumptions with PGO informed LTO,
but to my knowledge that's for stuff like speculative devirtualization. I
don't think it'd apply to anything like what we're dealing with here. But even
if it did, it'd not help with specializations, as the problem is having to
check all those branches in every IndexOnlyNext(), which in turn require a
bunch of loads.
> It would also be useful to measure the
> incremental gain with JIT expression compilation enabled, even
> though PostgreSQL's current JIT does not cover the scan loop itself.
> I'm also concerned about the cumulative code-size cost. The
> increase from roughly 0.6 KB to 3.5 KB may be acceptable for
> IndexOnlyScan alone, but applying the same approach to IndexScan,
> BitmapHeapScan, and other nodes could have broader implications
> for instruction-cache locality and maintainability.
Of course we shouldn't go overboard, but I'd expect this to *increase* icache
locality in a broad range of use cases. Postgres' binary is way too big and
our execution paths way too "wide" for relatively small increases in code size
to harm meaningfully, when the same changes make the amount of code executed
smaller in almost all cases. E.g. the EPQ handling code is never required in
hot execution paths, and keeping that on separate cachelines will *increase*
the actual icache effectiveness.
> Finally, with ongoing work on batched execution and index access,
> I think it would be worth understanding whether these optimizations
> remain complementary or primarily target the same per-tuple
> overhead. In particular, if batching amortizes executor dispatch
> and function-call costs, how much additional benefit would
> specialization provide?
I think batching in ExecIndexOnlyScan() is unlikely to be very generally
applicable anytime soon, because it's so often used in contexts like nested
loops, where - unless you have a much more complicated join implementation -
you will only want to fetch a single tuple on the inner side.
I also think we're not that close to actually getting the batched execution
stuff.
And more importantly, even with the batched execution, and a lot of additional
improvements, you'd *still* want specialization for having a qual, EPQ,
projection, because checking them inside the loop over the tuples in the batch
makes no sense. The only thing that'd actually be amortized is the indirect
function call to *Next().
Greetings,
Andres Freund
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Sami Imseih | 2026-10-14 19:32:44 | Re: pgstat: allow a stats kind to use its own dedicated dsa/dshash |
| Previous Message | Masahiko Sawada | 2026-10-14 19:14:01 | Re: Parallel autovacuum: honor reduced parallel workers after heap scan phase |