Cache client steady-state draw and hover work#193
Conversation
Merging this PR will not alter performance
Comparing 
Footnotes
|
Greptile SummaryThis PR reduces steady-state client rendering and hover work. The main changes are:
Confidence Score: 4/5The label invalidation and decoded-order hover paths need fixes before merging. In-place label configuration changes can leave stale DOM visible. Negative column scales can send descending decoded values through an ascending binary search. No security or destructive-operation issue was identified. js/src/50_chartview.ts
 T-Rex LogsWhat T-Rex did
Important Files Changed
Reviews (1): Last reviewed commit: "Cache client steady-state draw and hover..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| _labelDomStateSignature() { | ||
| const p = this.plot; | ||
| const signature = [ | ||
| this.spec, this.axes, this._themeEpoch, | ||
| this.size.w, this.size.h, p.x, p.y, p.w, p.h, | ||
| ]; | ||
| for (const axisId of this._axisIds()) { | ||
| const [lo, hi] = this._axisRange(axisId); | ||
| signature.push(axisId, lo, hi); | ||
| } | ||
| return signature; | ||
| } |
There was a problem hiding this comment.
When an incremental update mutates axis or annotation settings in place, changes to label text, tick formatting, rotation, or annotation content can preserve the object identities and axis ranges in this signature. labelsDirty then remains false, so the chart keeps stale label DOM until another captured property changes.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| g._cpuXSorted = true; | ||
| for (let i = 1; i < g.n; i++) { | ||
| if (!Number.isFinite(x[i]) || x[i] < x[i - 1]) { | ||
| g._cpuXSorted = false; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
This checks the encoded x column, but the new binary search orders decoded values using value / scale + offset. With a negative column scale, ascending encoded values become descending decoded values while _cpuXSorted remains true, so line hover can select the wrong row.
| g._cpuXSorted = true; | |
| for (let i = 1; i < g.n; i++) { | |
| if (!Number.isFinite(x[i]) || x[i] < x[i - 1]) { | |
| g._cpuXSorted = false; | |
| break; | |
| } | |
| } | |
| g._cpuXSorted = (g.xMeta?.scale ?? 1) > 0; | |
| for (let i = 1; g._cpuXSorted && i < g.n; i++) { | |
| if (!Number.isFinite(x[i]) || x[i] < x[i - 1]) { | |
| g._cpuXSorted = false; | |
| } | |
| } |
1 participant
Closes #171
Summary
Removes the four client-side steady-state costs identified in the issue:
ChartViewand invalidates them only onrefreshTheme()The committed interaction benchmark now exercises dashed lines and reports a no-state-change steady redraw metric. Focused real-browser tests cover cache invalidation, transition and context-loss fallbacks, GPU misses, pixel reuse, buffer cleanup, view/geometry invalidation, and label-DOM lifecycle.
Performance evidence
Fresh apples-to-apples Chromium/SwiftShader runs used the same updated harness on untouched
mainand this branch, 8 repetitions per interaction:All five interaction scenarios remained within their budgets with zero blank interaction frames and zero tick-label overlaps. Tooltip stability, box-zoom change/narrow/restore, brush selection/clear, and GL readback sanity checks all passed.
Verification
pytest -q tests/test_client_steady_state.py: 3 passed in real Chromiumtests/reflex_adapter/test_assets.py::test_client_source_is_the_installed_bundlestale literal-marker assertion; the same assertion was reproduced on untouchedmainnode js/build.mjs --check: committed bundles freshtsc -p js/tsconfig.json --noEmit: passedruff check .: passedruff format --check .: passedgit diff --check: passedThis PR is intentionally draft while the related performance-issue sweep is still in progress.