Skip to content

perf: SorterInsert reads GROUP BY sort key from source registers - #662

Merged
iheitlager merged 1 commit into
mainfrom
fix/656-hash-agg-allocations
Aug 30, 2026
Merged

perf: SorterInsert reads GROUP BY sort key from source registers#662
iheitlager merged 1 commit into
mainfrom
fix/656-hash-agg-allocations

Conversation

@iheitlager

Copy link
Copy Markdown
Member

Summary

Test plan

  • cargo test --lib — 976 passed
  • cargo test --test '*' — full integration/oracle-parity suite, no failures
  • cargo clippy --all-targets — clean
  • cargo bench --bench engine -- group_by_agg — before/after comparison shows measurable improvement, no regression on other Sorter-backed paths (ORDER BY unaffected, p5=0 default preserved)

spend: ~1x estimate (small), plus the #656 diagnosis pass that preceded it.

SorterInsert always re-decoded its sort-key columns out of the record
blob it had just been handed, even though those same values were still
sitting in registers moments earlier, before MakeRecord encoded them —
exactly the encode-then-decode overhead HashAggFind's own P3 parameter
already avoids for the (unused) hash-agg path. Profiling read_group_by_agg
found this redundant decode (decode_bytes_upto -> parse_header_into/
decode_serial_value) a meaningful share of per-row cost.

SorterInsert gains an optional p3 (source-register run) honored when p5
is nonzero; compile_grouped_scan's MakeRecord source registers are still
live at that point (record_reg is allocated after them), so it opts in.
Every other SorterInsert emitter (ORDER BY paths) leaves p5=0 and keeps
the original decode-from-blob behavior, so this is purely additive.

spend: ~1x estimate (small).

Refs #660

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@iheitlager
iheitlager force-pushed the fix/656-hash-agg-allocations branch from 6f445fa to 89ec86f Compare August 30, 2026 10:24
@iheitlager
iheitlager merged commit 7c50105 into main Aug 30, 2026
6 checks passed
@iheitlager
iheitlager deleted the fix/656-hash-agg-allocations branch August 30, 2026 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf: SorterInsert re-decodes GROUP BY sort-key from the record blob it just built

1 participant