Repository navigation
perf(call-tree): hold one Bottom-Up tree when Analysis is open too - #1260
Merged
lcottercertinia merged 2 commits intoOct 29, 2026
Merged
Conversation
…nalysis Both views built their own Bottom-Up tree for the same log, so a log open in both held every row twice. They now share one build per store. A caller that stops waiting gets null at once, and the build stops when no caller waits. On a 19 MB log, opening both views now peaks at about 450 MB. Refs certinia#1225.
lukecotter
force-pushed
the
bug-1225-shared-bottom-up
branch
from
October 29, 2026 11:43
fb6bc4d to
1632b3f
Compare
lcottercertinia
approved these changes
Oct 29, 2026
3 of 15 tasks
lcottercertinia
pushed a commit
that referenced
this pull request
Oct 29, 2026
… memory (#1262) # 📝 PR Overview After each Bottom-Up or Aggregated build, the tree walked every row to fill a parent map and a path map. On a 19 MB log that was 235 ms in one block, with 112 MB held for the two maps, before Analysis or the Call Tree could show. The build now records each row's parent and links rows of the same key as it makes them. Nothing walks the tree after the build. On the 19 MB log, Bottom-Up opens in 814 ms, not 989 ms, and holds 689 MB, not 777 MB. ## 🛠️ Changes made - Each row gets `_parent` from the build, so the tree needs no parent map. - Each row links to the next row with the same key. A highlight or reveal reads only the rows of that key, not the whole tree. A link in each row costs less memory than an array per key. - Reveal searches only the top-level rows for a path with no parent path, so it stays near 1 ms. - `marked` keeps its last answer. A view marks the same pick again on each pointer-leave, so that second call costs nothing. ## 🧩 Type of change (check all applicable) - [ ] 🐛 Bug fix - something not working as expected - [ ] ✨ New feature – adds new functionality - [ ] ♻️ Refactor - internal changes with no user impact - [x] ⚡ Performance Improvement - [ ] 📝 Documentation - README or documentation site changes - [ ] 🔧 Chore - dev tooling, CI, config - [ ] 💥 Breaking change ## 📷 Screenshots / gifs / video [optional] N/A, no UI change. ## 🔗 Related Issues related #1225 ## ✅ Tests added? - [x] 👍 yes - [ ] 🙅 no, not needed - [ ] 🙋 no, I need help - Built Bottom-Up and Aggregated trees: a nested row found by its path runs back up to its top-level row. - A path id the log never minted names no row. - `marked` keys the rows of every key in a pick, and a repeated pick returns the same answer without a second scan. ## 📚 Docs updated? - [ ] 🔖 README.md - [x] 🔖 CHANGELOG.md - [ ] 📖 help site - [ ] 🧪 Marked any pre-release-only features (README `🧪` badge — see [RELEASING.md](../RELEASING.md#-marking-pre-release-only-features)) - [ ] 🙅 not needed ## Anything else we need to know? [optional] - Follows #1260. The CHANGELOG line now gives the time and memory to open each tree, measured again against the code before #1258. - Measured on 8 logs, lowest of 3 runs: no log opens slower or holds more memory. - On the 19 MB log, highlighting the busiest frame (67k rows) takes 12.5 ms, against about 10 ms before. A repeated highlight takes 0 ms. - Tested with `pnpm test` (3364 tests).
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.
📝 PR Overview
The Call Tree's Bottom-Up tab and the Analysis view each built their own Bottom-Up tree for the same log. With both open, the webview held every row twice.
The two views now share one build for each log. On a 19 MB log, memory reaches about 450 MB when the first view opens, and stays there when the second one opens.
🛠️ Changes made
MergedTree.bottomUpkeeps one build per store and hands the same tree to every caller. The grid sorts and groups its own index arrays and never changes the rows, so each view keeps its own sort, grouping and selection.nullat once. If the build fails after that, the error does not reach this caller.🧩 Type of change (check all applicable)
📷 Screenshots / gifs / video [optional]
N/A, no UI change.
🔗 Related Issues
related #1225
✅ Tests added?
👍 yes
🙅 no, not needed
🙋 no, I need help
Each view has a test that its Bottom-Up rows are the shared tree's rows.
MergedTree.bottomUphas three tests:nullwhile the build finishes for another callerThe existing Call Tree detach tests still pass for Bottom-Up, so a detach still stops a build that nobody else waits for.
📚 Docs updated?
🧪badge — see RELEASING.md)Anything else we need to know? [optional]
pnpm test(242 suites, 3357 tests), and by hand in the dev host on the 🐛 bug: Bottom-Up build raises the heap by 1.2 GB on a 19 MB log #1225 log, with both views open.