perf(zql): probe the last view entry before binary-searching - #6508
Merged
Conversation
Hydration feeds the view its rows in the query's sort order, so every insert belongs at the end -- and `add` spent log2(n) comparisons per row rediscovering that, about nine for the 500-row benchmark. A Hermes CPU profile of `hydrate: issues only` attributes 55% of the whole benchmark to row comparisons made underneath this one binary search. Probing `view[length - 1]` first collapses those nine comparisons to one on the append path. It costs one extra comparison when the row does land inside the view, which is a single push rather than a bulk load. Measured on an Android emulator, median of 3 runs (rn-bench --repeat 3), together with the makeComparator change that follows: hydrate: issues only 189.70 -> 79.63 ms -58.0% hydrate: issues with creator 527.81 -> 412.05 ms -21.9% hydrate: issues with creator + comments 1035.63 -> 895.32 ms -13.5% hydrate: issues filtered open 162.38 -> 88.64 ms -45.4% hydrate: issues limit 50 18.19 -> 11.75 ms -35.4% push: add issue (no join) 14.84 -> 11.35 ms -23.5% push: add issue (with creator join) 25.83 -> 21.36 ms -17.3% push: add issue outside limit(50) 10.93 -> 11.58 ms +5.9% push: add comment (child relation) 32.43 -> 33.64 ms +3.7%
A single ascending field is what every source ordered by a single-column primary key gets, and it is the hottest function in hydration -- 16.5% self time in a Hermes profile of `hydrate: issues only`, more than compareUTF8 itself. Specializing it drops the loop, the direction test and the reverse test. The general path also swaps `for...of` for an indexed loop: Hermes allocates an iterator and calls next() per element, which for an ordering this short costs more than the comparison it is there to perform. Measured in isolation on Hermes (via rn-bench's hermes-eval): single string field -18.4% two fields -7.9%
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The optimizations preserve existing comparison and insertion semantics while improving the profiled hot paths.
Pull request overview
Optimizes ZQL IVM row insertion and comparison performance, especially during Hermes hydration.
Changes:
- Probes the final view entry before binary search to accelerate append-heavy hydration.
- Specializes single-field ascending comparators and replaces iterator-based comparison loops.
File summaries
| File | Description |
|---|---|
packages/zql/src/ivm/view-apply-change.ts |
Adds the append-path binary-search shortcut. |
packages/zql/src/ivm/data.ts |
Optimizes comparator creation and iteration. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Author
|
This one is so funny. |
aboodman
pushed a commit
that referenced
this pull request
Sep 10, 2026
Two changes to row comparison in the IVM view, found by profiling the ZQL benchmarks on Hermes (harness in #6507). ### Where the time goes I profiled `hydrate: issues only` on an Android emulator and attributed samples through the call tree rather than reading self-time. **55.5% of the entire benchmark is row comparisons made underneath one binary search** — `view-apply-change.ts`'s. The reason is structural: hydration feeds the view its rows in the query's sort order, so every insert belongs at the end. `add()` ran a full binary search anyway, spending log2(n) comparisons per row — about nine for the 500-row benchmark — to rediscover that. ### The changes 1. **Probe `view[length - 1]` before searching.** Nine comparisons become one on the append path. It costs one extra comparison when the row does land inside the view. This is not a heuristic that can be wrong: it's an early exit that returns exactly what the search would have. 2. **Specialize `makeComparator` for a single ascending field**, which is what any source ordered by a single-column primary key gets. That closure was 16.5% self time — more than `compareUTF8` itself. The general path also swaps `for...of` for an indexed loop; Hermes allocates an iterator and calls `next()` per element, which for an ordering this short costs more than the comparison. ### Measurements Android emulator, median of 3 runs (`rn-bench --repeat 3`), app data cleared between runs. Run-to-run spread was 0.5–6.4%. | benchmark | before | after | | |---|---|---|---| | hydrate: issues only | 189.70 | 79.63 | **−58.0%** | | hydrate: issues filtered open | 162.38 | 88.64 | **−45.4%** | | hydrate: issues limit 50 | 18.19 | 11.75 | **−35.4%** | | hydrate: issues with creator | 527.81 | 412.05 | −21.9% | | hydrate: issues with creator + comments | 1035.63 | 895.32 | −13.5% | | push: add issue (no join) | 14.84 | 11.35 | −23.5% | | push: add issue (with creator join) | 25.83 | 21.36 | −17.3% | | push: edit issue title | 35.52 | 35.81 | +0.8% | | push: add issue inside limit(50) | 15.60 | 15.63 | +0.2% | | push: add comment (child relation) | 32.43 | 33.64 | +3.7% | | push: add issue outside limit(50) | 10.93 | 11.58 | +5.9% | **The regressions are real and expected.** They're the extra comparison, on pushes where the row lands inside the view rather than at the end. They're small in absolute terms — a single push's search is microseconds — and two other push cases improve by 17–24% because their rows *do* land at the end. I think the trade is clearly right, but it is a trade and worth a look. I did not touch `shared/src/binary-search.ts`, which has the same shape. Its callers (Replicache subscriptions, the BTree, tdigest, flipped-join) aren't obviously append-heavy and I haven't measured them. ### Verification - All 1448 `zql` tests pass; `check-types` and `lint` clean. - `zero-client` has one failing test (`logged-out client uses a private storage sentinel for idb naming`). It fails identically with these changes reverted — pre-existing, not from this PR. ### A note on the numbers My first end-to-end measurement of change (1) said "no change". The benchmark runner only rebuilt the bundle under `--profile`, so it had measured the previous revision. The runner fix is in #6507. Usefully, the accident doubles as a harness sanity check: identical code measured 189.01 vs 189.73 ms.
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.
Two changes to row comparison in the IVM view, found by profiling the ZQL
benchmarks on Hermes (harness in #6507).
Where the time goes
I profiled
hydrate: issues onlyon an Android emulator and attributedsamples through the call tree rather than reading self-time. 55.5% of the
entire benchmark is row comparisons made underneath one binary search —
view-apply-change.ts's.The reason is structural: hydration feeds the view its rows in the query's
sort order, so every insert belongs at the end.
add()ran a full binarysearch anyway, spending log2(n) comparisons per row — about nine for the
500-row benchmark — to rediscover that.
The changes
Probe
view[length - 1]before searching. Nine comparisons become oneon the append path. It costs one extra comparison when the row does land
inside the view. This is not a heuristic that can be wrong: it's an early
exit that returns exactly what the search would have.
Specialize
makeComparatorfor a single ascending field, which is whatany source ordered by a single-column primary key gets. That closure was
16.5% self time — more than
compareUTF8itself. The general path alsoswaps
for...offor an indexed loop; Hermes allocates an iterator and callsnext()per element, which for an ordering this short costs more than thecomparison.
Measurements
Android emulator, median of 3 runs (
rn-bench --repeat 3), app data clearedbetween runs. Run-to-run spread was 0.5–6.4%.
The regressions are real and expected. They're the extra comparison, on
pushes where the row lands inside the view rather than at the end. They're
small in absolute terms — a single push's search is microseconds — and two
other push cases improve by 17–24% because their rows do land at the end. I
think the trade is clearly right, but it is a trade and worth a look.
I did not touch
shared/src/binary-search.ts, which has the same shape. Itscallers (Replicache subscriptions, the BTree, tdigest, flipped-join) aren't
obviously append-heavy and I haven't measured them.
Verification
zqltests pass;check-typesandlintclean.zero-clienthas one failing test (logged-out client uses a private storage sentinel for idb naming). It fails identically with these changes reverted —pre-existing, not from this PR.
A note on the numbers
My first end-to-end measurement of change (1) said "no change". The benchmark
runner only rebuilt the bundle under
--profile, so it had measured theprevious revision. The runner fix is in #6507. Usefully, the accident doubles
as a harness sanity check: identical code measured 189.01 vs 189.73 ms.