Repository navigation
crepe-plan-probe: ARM-B-PREPARE, Arm B's read policy above 1% of centers - #1639
Conversation
Arm B read the located lane ranges exactly at every selectivity. Above about 1% of centers its many short ranges cost Lance more per row than one sequential read (plan §2e). Each side now chooses its read with `addressed::plan_read`, a pure function of the located ranges and the table size: - exact ranges (touching ranges merge and stay exact); - coalesced ranges across gaps of at most `coalesce_gap` rows; - a full read once the ranges cover `full_permille` of the table. Rows read beyond the demand are dropped by the row's own leading value, so no position is taken from another lane's values. `ReadPolicy::EXACT` keeps the old behaviour and stays the default of `evaluate_addressed`; `evaluate_addressed_with` takes a policy. `ReadPolicy::DEFAULT` (gap 16, full at 100 permille) is pinned from examples/addressed_read.rs: - 50,000 rows: exact wins to about 3%, ties at 4-6%, loses from 10% (35.1 vs 18-19 ms at 50%). - 200,000 rows: exact wins to about 5%, ties at 7-10% (128.5 vs 74-79 ms at 50%). - Coalescing is neutral at gap 16 and loses at gap 64 on uniform demand. Tests: 5 unit tests on `plan_read`, B13-B15 against Lance (every policy answers as EXACT, the switch fires and stays silent per side, coalescing hands Lance fewer ranges). 46 pass. Disable runs, each red: no post-read drop, strict gap, never full, never coalesce, touching ranges counted as coalesced. Plan §2f, board entry 2026-10-20-arm-b-prepare, entries index. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VncKoyG6JkpsoqhqTGTv3f
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b03682176f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let wanted: Vec<u32> = u32::try_from(restricted(i).expect("restricted")) | ||
| .into_iter() | ||
| .collect(); | ||
| let (how, read_ranges, got, rows_io) = read_side(&ds, a, &r, &wanted, policy).await?; |
There was a problem hiding this comment.
When ReadPolicy::FULL or a coalescing policy is selected here, the unchanged SideIo construction still sets mask_rows to n, the number of rows in the original exact ranges. This violates SideIo::mask_rows's documented |R| meaning: a full read has no mask and coalescing masks additional gap rows. Consumers therefore see a nonexistent or undersized mask and cannot accurately compare it with rows_scanned/rows_returned; set it to None for Read::Full and to the merged-range span for masked reads in both side loops.
Useful? React with 👍 / 👎.
2 participants
Closes ledger (d) item ARM-B-PREPARE for uniform demand (#1636). Lab probe only; no production path changes.
Problem
Arm B reads the located lane ranges of every demanded key exactly, at any selectivity. Above about 1% of centers, its many short ranges cost Lance more per row than one sequential read (plan §2e): at 50% of centers, Arm B was slower than a plain full scan.
What changes
addressed::plan_readis a pure function of the located ranges and the table size; it reads no row. Per side it picks one of:Read::Ranges: the exact ranges. Ranges that touch are merged and still count as exact.Read::Coalesced: ranges at mostcoalesce_gaprows apart are merged.Read::Full: once the merged ranges coverfull_permilleof the table, the whole table is read without a row filter.ReadPolicyhas three constants:EXACTkeeps the old behaviour and stays the default ofevaluate_addressed.FULLalways reads the whole table.DEFAULTcoalesces gaps of up to 16 rows and switches to a full read at 100 permille.evaluate_addressed_withtakes a policy explicitly.Lookupnow reports the read chosen and the number of ranges handed to Lance.examples/addressed_read.rstimes every strategy by selectivity, after checking each one against the answer count. Environment variables:READ_N(table size),READ_KS(whichk),READ_REPS(runs).Measurement (release, uniform demand, ms)
The crossover moves with table size:
Between those points the choice is within run-to-run noise: one policy measured 3.9 and 6.1 ms at the same
kin one run.DEFAULTswitches at 10%, which is never measurably worse on either size.A full read has a floor of 3 ms at 50,000 rows and 6.7 ms at 200,000 rows, so it loses below the switch. Coalescing is neutral at gap 16 and loses at gap 64 (11.7 vs 6.9 ms at 1% of 200,000 rows). Full tables are in plan §2f.
Tests
5 unit tests on
plan_read:EXACTnever reads in full;FULLalways does.Against Lance:
EXACT, for every result kind, on a hit, a sponsor branch and a miss.46 tests pass. clippy (
-D warnings) and fmt are clean.Disable runs, each red:
B15 does not pin the drop: undemanded rows find no join partner there.
Open
DEFAULTby default.Board: plan §2f and follow-ups, entry
2026-10-20-arm-b-prepare, entries index. The supersession index is unchanged.Generated by Claude Code