perf(benchmark-nanoviews): take the row key and drop record - #218
Merged
Conversation
The row of the benchmark table needs two things from its item: the id, which is the key it is tracked by, and the label. It was reading both through `record`, so every row carried a proxy, two child signals, two effects and the links behind them - for one value that never changes and one that is read in a single place. The id now comes from `for_` as the row's key, and the label is read straight off the row. `record` earns its proxy when a row has several fields to write back or a field that changes far less often than the row it sits in; for one field that is only rendered, it does not. Against the app as it was: `01_run1k` script 24.70 ms -> 19.50, `07_create10k` 205.05 -> 181.60, `22_run-memory` 3.449 MB -> 2.943, and the bundle 12938 B -> 11982 as the whole `record` machinery leaves it.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #218 +/- ##
=======================================
Coverage 85.38% 85.38%
=======================================
Files 140 140
Lines 3155 3155
Branches 594 594
=======================================
Hits 2694 2694
Misses 332 332
Partials 129 129 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
The row of the benchmark table needs two things from its item: the
id, which is the key it is tracked by, and thelabel. It was taking both throughrecord:So every row carried a proxy, two child signals, two effects and the links behind them — for one value that never changes and one that is rendered in a single place.
The id arrives as the row's key (#217) and the label is read off the row.
Is dropping
recordfor the label safe?recordgives a child computed, so it can hold a text binding still when the row object changes but that field does not.() => $row().labelre-runs the binding on any change to the row.In this app that saving never happens:
partialUpdatebuilds a new object withassignKeyfor every tenth row only, so the hundred rows that get a new object are exactly the hundred whose label changed. The other nine hundred keep their identity and the reconcile does not write to them at all. The measurement agrees —03_update10th1k_x16, the one case where the dedup could have paid off, got slightly faster: 3.00 ms → 2.90, CI [−0.30; −0.05].recordstill earns its proxy when a row has several fields to write back, or a field that changes far less often than the row it sits in. For one field that is only rendered, it does not.Measured
Two changes, measured separately, each with 30 iterations per arm and two rounds with the arms alternating.
recordThe app bundle goes 12938 B → 11982: with the last consumer gone, the whole
recordmachinery leaves it.Together with #216 the weighted geometric mean moves 1.139 → 1.103, which puts nanoviews ahead of arrow-js. Computed conservatively: only the measured
scriptdeltas are taken off the reference totals, since none of this touches paint and07's paint swings by ±40 ms between sessions.