fix(perf): rebuild the list-index map lazily on read instead of per operation#2802
Draft
christianhg wants to merge 1 commit into
Draft
fix(perf): rebuild the list-index map lazily on read instead of per operation#2802christianhg wants to merge 1 commit into
christianhg wants to merge 1 commit into
Conversation
…peration `listIndexMap` (the source of `data-list-index` on rendered list items) is derived from the whole value: list-item numbering depends on block adjacency, so any structural operation could change it. The update-value subscriber rebuilt it from scratch after every operation whose path was at most two segments deep, walking all root blocks each time. A bulk edit applying N root operations therefore did O(N) full walks, i.e. O(N^2), even though nothing reads the map until the next render, and the `path.length <= 2` gate silently assumed list numbering can only live at the document root. The only reader is the text-block renderer. Structural operations now just mark the map dirty (on any path, dropping the depth assumption) and `getListIndexMap` rebuilds it on the next read. A batch of operations collapses to a single rebuild per render regardless of size, and the map is recomputed for structural changes at any depth. Net behavior unchanged: the rendered `data-list-index` values are identical; only when the rebuild happens moved from per-operation to per-read.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 4e4ded3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
📦 Bundle Stats —
|
| Metric | Value | vs main (525dac0) |
|---|---|---|
| Internal (raw) | 786.1 KB | +107 B, +0.0% |
| Internal (gzip) | 150.1 KB | +79 B, +0.1% |
| Bundled (raw) | 1.39 MB | +115 B, +0.0% |
| Bundled (gzip) | 311.9 KB | +72 B, +0.0% |
| Import time | 95ms | -0ms, -0.2% |
@portabletext/editor/behaviors
| Metric | Value | vs main (525dac0) |
|---|---|---|
| Internal (raw) | 467 B | - |
| Internal (gzip) | 207 B | - |
| Bundled (raw) | 424 B | - |
| Bundled (gzip) | 171 B | - |
| Import time | 2ms | +0ms, +0.7% |
@portabletext/editor/plugins
| Metric | Value | vs main (525dac0) |
|---|---|---|
| Internal (raw) | 2.7 KB | - |
| Internal (gzip) | 894 B | - |
| Bundled (raw) | 2.5 KB | - |
| Bundled (gzip) | 827 B | - |
| Import time | 7ms | +0ms, +1.0% |
@portabletext/editor/selectors
| Metric | Value | vs main (525dac0) |
|---|---|---|
| Internal (raw) | 78.5 KB | - |
| Internal (gzip) | 14.4 KB | - |
| Bundled (raw) | 74.0 KB | - |
| Bundled (gzip) | 13.3 KB | - |
| Import time | 8ms | +0ms, +1.4% |
@portabletext/editor/traversal
| Metric | Value | vs main (525dac0) |
|---|---|---|
| Internal (raw) | 24.6 KB | - |
| Internal (gzip) | 4.9 KB | - |
| Bundled (raw) | 24.5 KB | - |
| Bundled (gzip) | 4.8 KB | - |
| Import time | 6ms | +0ms, +2.9% |
@portabletext/editor/utils
| Metric | Value | vs main (525dac0) |
|---|---|---|
| Internal (raw) | 28.8 KB | - |
| Internal (gzip) | 6.0 KB | - |
| Bundled (raw) | 26.7 KB | - |
| Bundled (gzip) | 5.7 KB | - |
| Import time | 6ms | -0ms, -0.6% |
🗺️ . · ./behaviors · ./plugins · ./selectors · ./traversal · ./utils · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — @portabletext/markdown
Compared against main (525dac02)
| Metric | Value | vs main (525dac0) |
|---|---|---|
| Internal (raw) | 53.0 KB | - |
| Internal (gzip) | 9.6 KB | - |
| Bundled (raw) | 347.6 KB | - |
| Bundled (gzip) | 96.0 KB | - |
| Import time | 39ms | +0ms, +1.2% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
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
1 participant
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.
Editing a document rebuilt
listIndexMapfrom scratch on every structural operation, walking the entire value each time, so a batch of N operations did N full O(value) walks before React ever rendered. Nothing reads the map until render, and the rebuild was gated onoperation.path.length <= 2, silently assuming list-item numbering can only live at the document root.listIndexMap(the source ofdata-list-indexon rendered list items) is consumed only by the text-block renderer, never by operations or selectors. Structural operations now just mark it dirty (on any path, dropping the depth assumption) andgetListIndexMaprebuilds it the next time the renderer reads it. A batch of operations collapses to one rebuild per render regardless of size, and the map stays correct for structural changes at any depth, not just the root.Net behavior is unchanged: the rendered
data-list-indexvalues are identical; only when the rebuild happens moved from per-operation to per-read.This is one of two extracted performance PRs. The other (
editor-perf-dedupe-key-scan) removes a separate per-operation O(n) rescan from duplicate-key normalization; the two target different costs and are additive. They touch the same engine fields (subscribeUpdateValue, the engine type, engine setup), so whichever merges second needs a trivial rebase.Chromium, headless, medians of 3 runs,
tests/performance.test.tsx, this change alone vsmain:The larger win shows on list-heavy or large documents, where the per-operation full walk dominated; the generic insert benchmark only lightly exercises it.