Repository navigation
perf(join): skip speculative hashing for minimum chained tables - #942
Merged
Merged
Conversation
…n-min-heads-current
…n-min-heads-current
singaraiona
approved these changes
Oct 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Landed normally
02804c3: independent Anton approval of the prefetch feature preserved through normal reviewed-base imports; fresh currentf7/head/APPROVED/resolved/CLEAN/MERGEABLE and all11 actual COMPLETED/SUCCESS gates before ordinary protected expected-SHA squash. Complete checked/landed tree f2f264a91d1fb7c8bdddbf5b8e931f1a690d3eb0 identical; remote dev verified. No master/tag/release action. No pool-policy/index-routing issue auto-closed (closing references checked empty).
Combined-head local join114/identity3at0/4helpers and other scopes below retain their actual provenance; earlier prefetch-only native measurements are not remeasured whole-combined-head prices.
Current accepted-identity-LEFT integration —
41abae79Normal expected-head import of accepted #943 /f7b23ec7. Ordered parents/computed complete merge tree verified. join.c combines accepted #943 output-retention and per-column slice fallback; all5other feature blobs unchanged. Complete incoming join.c delta personally read; the prefetch feature is preserved, not a byte-identical whole join.c. Fresh non-root ALL-TU ClangASan/UBSan join114, no-helper pair1, affine20, zeroRFL2, imported identity3each at0/4configured helpers, TIME3/metadata2 passed. GCC DEBUG/NDEBUG syntax2, whole relevant logs diagnostic0, exact clean source/both binaries checked. Compared with accepted #943s113, the sole additional selected case is rfl/join/tiny_chained_prefetch. No new local full/TSan/combined-head native/backtester claim. Fresh actual exact-head/current-base/review/discussion/gates remain required before landing.
Prior TIME32 integration —
cee4db5fNormal expected-head import of accepted #939 /5f6e619b. Ordered parents/computed whole merge tree verified; all6feature blobs unchanged. Fresh non-root ALL-TU/header-consistent ClangASan/UBSan join110, no-helper pair1, affine20, zero-divisor2, imported TIME3 and metadata2 passed. GCC changed production TU DEBUG/NDEBUG syntax2, whole relevant logs diagnostic0, exact clean head/both binaries checked. No new local full/TSan/native/backtester claim; earlier evidence retains its named heads. Fresh exact-head CI/current-base/reviews/threads remain required before ordinary landing.
Prior B0 integration —
bf022ae7Normal expected-head import of accepted #936 / b0b0a1b. Ordered parents and computed complete merge tree verified; all6feature blobs unchanged. Fresh non-root ALL-TU/header-consistent ClangASan/UBSan join110, no-helper pair1, affine20, zero-divisor2 and imported metadata2 passed; GCC changed production TU DEBUG/NDEBUG syntax2, whole relevant logs diagnostic0, exact clean head/both binaries verified. No new local full, TSan, native pricing or author-backtester claim. Earlier measurements/controls below retain their original heads. Fresh exact-head CI is still required.
Prior D715 integration —
a1a4bcb7Anton independently approved 5bbe455. Normal import of accepted d7153e7 / #937 givesa1a4bcb7: ordered parents/computed complete merge tree verified, all6feature blobs unchanged, including join.c/fixture/benchmark files. Fresh non-root all-TU/header-consistent ClangASan/UBSan join110/110, no-helper pair1/1, affine20/20, zero-divisor2/2, imported typed-sort probe1/1; GCC join DEBUG/NDEBUG syntax2. Whole relevant logs diagnostic0, exact clean head/both binaries checked. This does not relabel earlier full/Tsan/native measurements as new executions. Waiting for current head’s actual required checks; no self-approval or bypass.
Why
The chained join's count and fill passes hash row
i+8again merely to prefetch a bucket head. With the minimum256-head array (1KiB), that speculative work is expensive on the small-build workload measured here.Change
The gate uses capacity, not distinct-key count: right128/129 selects256/512heads. A small head array does not prove the entire working set resident.
Measured, bounded release result
Matched Clang18O3/native/i7-6700 comparison: immutable accepted
bda2f218release, replace only its join object with the exact candidate source; common objects and flags/hash proofs retained. This is not a fresh whole-final-head release binary. The pre-#943 candidate matched the measured TU. Current combined join.c additionally contains accepted #943; earlier prices are not whole-combined-head measurements.Fixed130k-row/773-symbol composite-key input,200 inner joins against3unique keys, independent count101400/key checks; selected-core affinity, no concurrent local builds/tests, randomized fresh processes/eight samples after separate discarded warmups. Requested participants are not instrumented worker participation.
A three-way162-process comparison also priced the simpler row-mode-test version: callback selection improves its one-participant loop854.914→833.265ms; four-participant255.175→249.971ranges overlap. Inspected Clang objects have the mode-specific prefetch eliminated/retained as intended; object text grows1240bytes. Not a universal compiler guarantee.
Retained-prefetch controls are not hidden: SYM129 candidate loop282.951/88.069ms versus baseline281.815/87.970, longSTR1292151.520/691.559 versus2159.760/691.727 (1/4participants); ranges overlap, including a large serialSYMoutlier. The earlier simpler candidate's broader11-case study had a radix-size65537/four-participant+4.8%median with overlapping ranges; that is not final-version radix pricing. No universal neutrality, new instruction-count result, exclusive-host or whole-engine speed claim.
Correctness and provenance
adb16ece, same runtime/fixture blobs: forced join110/110; full non-root ClangASan/UBSan 6419/6436passed,17skipped,0failed. All1488tracked-source manifests/both binaries/HEAD/index guards and independent whole-log diagnostic checks match; binaries preserved.5bbe4556imports accepted5f44d2d1, then changes only benchmarkREADME. Computed merge tree and all six feature blobs before that docs-only edit match. Fresh consistent-header all-TU final-head Clang ASan/UBSan: join110/110, no-helper pair fixture1/1, imported affine20/20 and zero-divisor2/2. GCC join.c DEBUG/NDEBUG syntax2pass; forced current scopedTSan pair fixture1/1 with4background helpers configured (participation uninstrumented). Whole relevant logs diagnostic0, exact clean head/canonical identity/both normal binaries checked. This is not a second full-final-head run.bench/join_tiny_htsupplies a fixed-key probe and standard-library Python process-comparison wrapper; preserve separate release binaries (test builds overwrite the CLI). This is related to the workload discussed in#599, not a scheduler-policy fix or resolution of that issue. No spill/domain/cancellation contract expanded or allocator-failure injection claim. Independent maintainer review and fresh required CI remain necessary.