| From: | Haibo Yan <tristan(dot)yim(at)gmail(dot)com> |
|---|---|
| To: | Matthias Dötsch <matze(at)mdoetsch(dot)de> |
| Cc: | Andres Freund <andres(at)anarazel(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-15 18:03:00 |
| Message-ID: | CABXr29EzS-b6=ygYvp7hz9+xkcco1Mtj4MboPAdfXs6xUCk=zw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Oct 15, 2026 at 12:29 AM Matthias Dötsch <matze(at)mdoetsch(dot)de> wrote:
>
> Hi Andres, Haibo,
>
> thanks for the explanations and for testing this.
>
> On Wed, 14 Oct 2026 19:01:43 -0700, Haibo Yan wrote:
> > With default alignment, the qual-only case improved latency by
> > roughly 5-6%. With 64-byte alignment, however, the improvement
> > disappeared.
>
> I see the same here, only the direction differs per query. Same queries
> as before, master vs patch, default alignment and -falign-functions=64
> (i7-13700H, gcc 12):
>
> instructions wall-clock wall-clock
> default align 64
> no qual, no projection -5.2% -10.9% -5.1%
> with qual -4.8% -5.2% -2.8%
> with projection -6.3% -4.4% -10.4%
>
> So with 64-byte alignment the qual case shrinks like in your run, and
> the projection case gets bigger. Your SeqScan control probably explains
> it, the patch moves everything behind it in the binary. I'd rather go
> by the instruction counts, those are stable and match between our two
> machines.
>
> > In particular, it would be interesting to see how much of the
> > improvement survives with LTO/PGO enabled.
>
> I tried -Db_lto=true (gcc 12, -flto=auto):
>
> master+LTO patch+LTO diff
> no qual, no projection 637,092,111 603,092,078 -5.3%
> with qual 659,425,866 626,759,171 -5.0%
> with projection 645,088,506 603,088,469 -6.5%
>
> LTO alone gives about 2.7% on master for me, and the patch still saves
> the same ~5% on top. nm on the LTO master binary still shows ExecScan()
> and IndexOnlyNext() as seperate functions, so I guess LTO didn't inline
> anything there, which would match what Andres said.
>
> JIT: LLVM 15, jit=on, jit_above_cost=0, jit_optimize_above_cost=0. To
> get the compile cost out of the callgrind numbers I ran the same
> queries with 1 row and subtracted that. The patch then saves 34.0M /
> 32.7M / 42.0M instructions, the same as without JIT to within a hundred
> instructions, which makes sense since it doesn't touch the JIT compiled
> parts at all. Net of the compile that's -5.6% / -5.3% / -6.7%.
> Wall-clock with JIT: -9.2%, -8.1%, -5.8%.
>
> Andres mentioned nested loops, so I also tried that out: 1M outer rows,
> inner index only scan on the pkey of the 5M row table, one match each.
> Instructions -0.4% without and -0.6% with a filter on the inner side,
> wall-clock within noise (~1.3 s per query, rounds vary by about 5%).
> Makes sense I think, the lookup is ~9400 instructions and almost all of
> it is rescan and btree, ExecScan() is only 30-50 of them.
>
> > I'd appreciate your thoughts on whether the attached EPQ isolation
> > test would be useful to include.
>
> Yes, I think so. It passes here on master and with the patch, and CI on
> my github fork is green with it. With a temporary elog() in
> ExecIndexOnlyScanEPQ() I could see that it really goes through that
> function. I attached it as 0001 of v3 with you as author, tell me if
> this is the usual flow here.
>
> Is there anything else you'd want to see tested or benchmarked before
> this could move on? Other workloads, other compilers, more EPQ cases?
> Happy to run more, I'm just not sure which cases matter most.
>
> Regards,
> Matthias
Hi Matthias,
Thanks for the additional benchmarks and for including the EPQ
isolation test in v3.
The LTO and JIT results are useful. They confirm Andres' point
that specialization provides benefits that compiler optimization
and the current JIT implementation cannot eliminate.
The nested-loop results also make sense. When most of the work
is spent on rescans and B-tree lookups, reducing the ExecScan()
overhead can only have a limited impact.
Regarding the authorship of the test patch, I'm not entirely
familiar with the usual community conventions. Since you're
maintaining the complete patch series, I'm happy for you to
use your own name for 0001. I don't need separate author credit
for this test.
As for further testing, I think we've covered the main areas
I was concerned about. The correctness checks, EPQ coverage,
and performance results look sufficient to me at this point.
The CI results are also encouraging.
I haven't found anything that would justify holding up v3
for additional benchmarking. I'd be inclined to let the
normal review process continue and see if other reviewers
identify anything that needs further investigation.
Thanks again for following up on all these points.
Regards,
Haibo
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Greg Burd | 2026-10-15 18:15:08 | Re: Tepid: selective index updates for heap relations |
| Previous Message | Jim Jones | 2026-10-15 17:46:11 | Re: Unlogged materialized views |