Skip to content

perf(join): retain left columns for identity output mappings - #943

Merged
morsel-ray merged 4 commits into
RayforceDB:devfrom
vbmithr:perf/identity-left-join
Oct 22, 2026
Merged

morsel-ray merged 4 commits into
RayforceDB:devfrom
vbmithr:perf/identity-left-join

Conversation

@vbmithr

@vbmithr vbmithr commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

A flat LEFT join currently allocates and gathers every left column even when its output contains exactly the original left rows in their original positions. This change verifies that completed mapping and retains unchanged ordinary columns, avoiding their allocation, gather and metadata propagation. Slice-valued columns keep the independent gather path because slice-header copy-on-write does not detach their shared parent; other columns in the same table can still be retained.

The check requires join_type == LEFT, pair_count == left_rows, and l_idx[i] == i for every output row. Checking cardinality alone would be unsafe because radix partitioning can reorder even one-to-one joins. Duplicate expansion, reordered mappings and FULL joins keep the existing gather path. Shared columns keep their null bitmaps, string pools and symbol domains, and subsequent updates retain copy-on-write isolation. No sort code is changed.

Includes C ownership and lifetime regressions, Rayfall value/update regressions, join documentation, and a standalone benchmark at bench/join_identity/.

Performance

Matched release builds from d7153e72, identical flags and Clang 23.1.3, changing only join.c. Hourly 100-instrument xsmomf backtest: 26,304 steps, 2,630,400 bars, 705 fills; the same already projected report in both variants. One discarded warm-up each, then three fresh processes each in shuffled order (seed 22), without concurrent local builds or tests:

Median Base Candidate
Post-run checks 1146.46 ms 1047.97 ms (-8.6%)
Whole process 4.137 s 4.022 s (-2.8%)
Peak RSS 1608.2 MiB 1590.1 MiB (-18.1 MiB)

These measurements include the slice fallback correction. Check ranges were 1134.08–1163.55 ms versus 1043.04–1051.07 ms. This is a small experiment on a shared host, not a general speedup claim. The backtester/data are external; the standalone probe needs neither and verifies values on both base and candidate.

Validation

  • 113/113 join-related tests under ASan/UBSan, four secondary workers.
  • All three identity/ownership regressions also pass under ASan/UBSan with workers disabled.
  • Slice regression first reproduced the original PR failure: an owned output mutation changed its source. The corrected test checks source slice, parent and original result values, output lifetime, and continued sharing of the ordinary column in the same table.
  • Equal-cardinality radix fixture verifies actual reordering, unique keys and matching left/right payloads. Deliberately removing the identity guard fails the left/right payload-association assertion; restoring it passes.
  • Existing Rayfall regression passes: nulls, symbols, nested lists, source/result update isolation, unmatched rows, duplicate expansion and FULL joins.
  • Backtester fixtures 1–6, vector/reference equivalence and independent accounting oracle pass again (50 seeds, three instruments, 16 steps, REJECT).
  • Standalone benchmark correctness checks pass on the corrected release binary.
  • git diff --check clean.
  • The previous-head full local run was interrupted before rebuilding for this correction; no full-suite pass is claimed. Full CI is pending for the new head.

Related to #596's output-materialization discussion, but preserves table output and adds no mask API. Complementary to #942's speculative-hashing optimization; does not implement the index-routing requested in #595.

Verify the completed left index mapping before sharing unchanged columns
in flat LEFT joins. Retain their existing nulls, string pools and symbol
domains without gathering or rewriting metadata on shared objects.
Keep duplicate expansion, reordered output and FULL joins on the gather
path, and preserve copy-on-write isolation for subsequent updates.

Cover ownership, source lifetime, nulls, strings and nested values, plus
large joins. Add a standalone probe and matched backtester measurements.

Co-Authored-By: GPT-6 <noreply@openai.com>

@morsel-ray morsel-ray left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read the full production/test/docs/benchmark delta. The mapping-based admission is sensible, but I independently reproduced a public C mutation-isolation regression for a valid slice-valued left payload. Details inline. This is an existing slice-COW limitation newly exposed by replacing a gather with retention, not an observed crash or a claim that Rayfall update fails. My paired native probe changes only join.c against current D715 common objects; no independent full/sanitizer/benchmark claim.

Comment thread src/ops/join.c Outdated
Comment thread test/test_join_buildside.c
Keep slice-valued left columns on the independent gather path because
copy-on-write currently copies only their headers and can mutate the
shared parent payload. Retain eligible ordinary columns in the same
table, and propagate metadata only for newly gathered output columns.

Reproduce the owned C mutation and assert source, parent and original
result values. Pin equal-cardinality reordered radix output with matched
left/right payloads, and verify removing the mapping guard fails it.
Refresh the benchmark measurements after the slice fallback correction.

Co-Authored-By: GPT-6 <noreply@openai.com>

@morsel-ray morsel-ray left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved exact6217b31d after independently verifying both requested changes. Read full original production/test/docs/benchmark delta and the complete correction. My paired public-C slice probe now isolates source/parent/original output while ordinary columns still retain; actual fresh all-TU ClangASan/UBSan join113/113, identity3/3 at0and4background helpers and GCC DEBUG/NDEBUG syntax2 passed. Relevant whole logs diagnostic0, clean exact source/both binaries verified and snapshots preserved. A compiled mapping-guard reversal hits the new radix left/right payload-association assertion; exact restore passes3/3 (the deliberate failing fixture has a test-local LSan abort, not a production finding). No independent full-suite, TSan, backtester, exclusive-host performance or every-representation clearance. Author measurements remain attributed; the correction retains the bounded optimization. Current-base import and fresh exact-head required checks are still needed before ordinary landing. No gate bypass.

@morsel-ray

morsel-ray commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Landed normally

f7b23ec: fresh exact-head/current5f6base, APPROVED/resolved discussions/CLEAN/MERGEABLE and all11 actual COMPLETED/SUCCESS gates before ordinary protected expected-SHA squash. Complete checked/landed tree c0bc22238904682e9ed2d34b2355019d896cd334 identical; remote dev verified. No master/tag/release action.

Local importede8a2e56a all-TU ASan/UBSan join113, identity3each at0/4configured background helpers, imported TIME3/metadata2, GCC2 and clean source/binary/whole-log guards retain their evidence. My exact6217approval and paired slice-COW/mapping-guard controls remain at their recorded provenance, not new full/TSan/backtester/performance clearance.

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.

2 participants