perf(pl-tree): cut the cost of applying a tree update - #1813
Open
xnacly wants to merge 4 commits into
Open
Conversation
🦋 Changeset detectedLatest commit: 6cfda51 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 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 |
xnacly
force-pushed
the
perf/tree-apply-cost
branch
from
September 9, 2026 11:09
9c36c85 to
be05816
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1813 +/- ##
==========================================
- Coverage 54.54% 54.52% -0.02%
==========================================
Files 418 418
Lines 21936 21936
Branches 4913 4913
==========================================
- Hits 11964 11960 -4
+ Misses 8533 8532 -1
- Partials 1439 1444 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
xnacly
force-pushed
the
perf/tree-apply-cost
branch
2 times, most recently
from
September 9, 2026 12:19
b07d0e4 to
d56e597
Compare
xnacly
marked this pull request as ready for review
September 9, 2026 12:24
xnacly
added this pull request to stack #1815
September 9, 2026 12:31
markChanged detaches every watcher on its first call and nothing can attach in the middle of the synchronous patch loop, so notifying once per new resource was already a no-op from the second resource on, while still building a marker string whose resourceIdToString parses a BigInt and hex-decodes a Buffer. 21% faster on a first load of 10k resources; no effect on a steady poll, which adds no resources.
…y name Every field name in a poll response is freshly decoded from protobuf, so V8 has no cached hash for it and fieldsMap.get had to compute one per field: profiling put that at 47% of this method's self time. Stored fields are now walked in step with the incoming ones and matched by direct comparison, and any divergence abandons the walk and falls back to the hash lookup. 10% faster on a steady poll of 10k resources. New tests cover the divergence shapes.
Same technique and same reason as the field walk: kv keys arrive in a stable order and are freshly decoded strings, so resource.kv.get was hashing each one. The length guard on both walks also avoids allocating an iterator for a resource that carries no kv at all, which is the common case. 2% faster on a steady poll of 10k resources.
A closure and a full basicState clone were allocated for every resource in every update purely so a fatal-path message could quote the pre-update state. Only the mutable half of BasicResourceData needs capturing; id, kind, type and data are readonly on PlTreeResource and are read back when the error is built, so the message text is unchanged. 4% faster on a steady poll of 10k resources.
xnacly
force-pushed
the
perf/tree-apply-cost
branch
from
September 9, 2026 13:35
d56e597 to
6cfda51
Compare
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.
Makes
PlTreeState.updateFromResourceData, the apply step of a tree-sync poll, faster.Changes
5126c71NotifyresourcesAddedonce per update instead of once per new resource.→ 21% faster on a first load, nothing on a steady poll.
f3d0776Walk stored fields in lockstep with the incoming ones, matching by direct namecomparison instead of a
fieldsMaphash lookup per field — poll-response field names arefreshly decoded, so V8 has to hash every one. → 10% faster on a steady poll.
c64783aSame lockstep walk for KV entries. → 2% faster on a steady poll.d56e597Hoist the transition-error snapshot out of the patch loop, dropping a closureand a
basicStateclone per resource per update. → 4% faster on a steady poll.Improvement
Per apply, at 10,000 resources with ~10 fields and 2 KV entries each:
collectGarbageGreptile Summary
This PR optimizes the in-memory apply phase of tree synchronization while retaining existing field, KV, reference-count, watcher, and transition-validation behavior.
Important touched terms
PlTreeState.updateFromResourceData— Applies incoming resource snapshots to the local tree. It now batches resource-added notification, uses lockstep field/KV traversal, and reuses transition-error diagnostic machinery.PlTreeResource— The stored representation of one resource and its mutable state. Its pre-update state is now captured through scalar diagnostic variables rather than cloningbasicStatefor every resource.fieldsMap— A name-keyed map of a resource’s fields. Incoming fields now take a direct iterator fast path while order matches, then fall back to map lookup after any divergence.resource.kv) — Per-resource string keys mapped to binary values. Their synchronization now uses the same lockstep fast path, abandoning it before inserting into the map.resourcesAdded— The change source watched when a requested resource is absent. It is now marked once after an update rather than once for every newly created resource.unexpectedTransitionError— Invalidates the tree and reports an illegal resource-state transition. Its closure and immutable diagnostic setup are now allocated once per update.Additional changes
Confidence Score: 5/5
The PR appears safe to merge, with the optimized paths preserving existing observable behavior and receiving focused regression coverage.
No actionable correctness, security, or repository-rule issue remains; exact-name fallback protects divergent field and KV ordering, batched notification preserves watcher semantics, and the diagnostic snapshot matches the prior pre-mutation state.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Incoming resource snapshots] --> B{Stored resource exists?} B -- No --> C[Create resource] C --> D[Count newly added resource] B -- Yes --> E[Capture mutable pre-update state] E --> F[Apply metadata and state transitions] F --> G[Walk stored fields in lockstep] G --> H{Names still match?} H -- Yes --> I[Use iterator value directly] H -- No --> J[Fall back to fieldsMap lookup] I --> K[Apply field changes and reference deltas] J --> K K --> L[Walk KV entries in lockstep] L --> M{Keys still match?} M -- Yes --> N[Use iterator value directly] M -- No --> O[Fall back to KV map lookup] N --> P[Apply KV changes and deletions] O --> P D --> Q[After patch loop] P --> Q Q --> R[Notify resourcesAdded once] R --> S[Apply reference deltas and garbage collection]Reviews (1): Last reviewed commit: "perf(pl-tree): hoist the transition-erro..." | Re-trigger Greptile