[Tree widget]: reduce cache coupling on next#1726
Open
JonasDov wants to merge 9 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the tree widget ID caching layer to avoid triggering sub-model (modeled elements) queries during initial load, improving performance in presentation-performance-tests and simplifying cache responsibilities.
Changes:
- Decouples category/model-category caching from modeled-elements caching, and moves several “re-export” APIs into
BaseIdsCache. - Makes modeled-elements caching lazy and filters modeled-elements queries via
IdSetbindings for improved query performance. - Updates tree definitions/visibility helpers and tests to align with the new cache APIs and data flow.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/itwin/tree-widget/src/tree-widget-react/components/trees/models-tree/ModelsTreeDefinition.ts | Replaces top-most category aggregation with per-model cache lookups (avoids coupling to sub-model caching). |
| packages/itwin/tree-widget/src/tree-widget-react/components/trees/models-tree/internal/ModelsTreeIdsCache.ts | Updates category-to-model path resolution to use new getModels filtering options. |
| packages/itwin/tree-widget/src/tree-widget-react/components/trees/common/internal/visibility/BaseVisibilityHelper.ts | Adapts visibility logic to new cache APIs (getModels, getSubModels, categoryHasParentElements). |
| packages/itwin/tree-widget/src/tree-widget-react/components/trees/common/internal/caches/SubCategoriesCache.ts | Removes now-unneeded getSubCategories wrapper API. |
| packages/itwin/tree-widget/src/tree-widget-react/components/trees/common/internal/caches/ModeledElementsCache.ts | Uses IdSet binding to constrain modeled-elements query input set. |
| packages/itwin/tree-widget/src/tree-widget-react/components/trees/common/internal/caches/ElementModelCategoriesCache.ts | Removes dependence on modeled-elements cache and trims exported API surface to cached-data access. |
| packages/itwin/tree-widget/src/tree-widget-react/components/trees/common/internal/caches/BaseIdsCache.ts | Centralizes cache APIs, adds lazy modeled-elements cache construction, and re-implements removed helpers. |
| packages/itwin/tree-widget/src/tree-widget-react/components/trees/classifications-tree/internal/visibility/ClassificationsTreeVisibilityHelper.ts | Switches to getAllCategoriesOfElements({ onlyTopMostElementCategories: true }). |
| packages/itwin/tree-widget/src/tree-widget-react/components/trees/categories-tree/internal/visibility/CategoriesTreeVisibilityHelper.ts | Adapts to getModels() now emitting modelId values directly. |
| packages/itwin/tree-widget/src/tree-widget-react/components/trees/categories-tree/internal/CategoriesTreeIdsCache.ts | Refactors sub-category search paths to use new getCategoryId API. |
| packages/itwin/tree-widget/src/tree-widget-react/components/trees/categories-tree/CategoriesTreeDefinition.ts | Mirrors models-tree change: per-model category aggregation and updated getModels usage. |
| packages/itwin/tree-widget/src/test/trees/models-tree/internal/ModelsTreeIdsCache.test.ts | Updates expectations after removing getElementsCount in favor of getDescendantsCounts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Tree-Widget Next benchmark
| Benchmark suite | Current: deb72ab | Previous: 424b2c2 | Deviation | Status |
|---|---|---|---|---|
models tree 50k 3D elements search > get search paths |
1227 ms |
1205 ms |
1.83% |
〰️ |
models tree 50k 3D elements search > get search paths (P95 of main thread blocks) |
46 ms |
58 ms |
-20.69% |
〰️ |
models tree 50k 3D elements search > load hierarchy from search paths |
114638 ms |
114312 ms |
0.29% |
〰️ |
models tree 50k 3D elements search > load hierarchy from search paths (P95 of main thread blocks) |
36 ms |
36 ms |
0% |
🟰 |
models tree 50k categories > collect nodes |
2987 ms |
2976 ms |
0.37% |
〰️ |
models tree 50k categories > collect nodes (P95 of main thread blocks) |
113 ms |
94 ms |
20.21% |
〰️ |
models tree 50k categories > validate initial visibility |
1804 ms |
1737 ms |
3.86% |
〰️ |
models tree 50k categories > validate initial visibility (P95 of main thread blocks) |
0 ms |
0 ms |
0% |
🟰 |
models tree 50k categories > change visibility |
212 ms |
211 ms |
0.47% |
〰️ |
models tree 50k categories > change visibility (P95 of main thread blocks) |
61 ms |
59 ms |
3.39% |
〰️ |
models tree 50k categories > validate changed visibility |
2370 ms |
2277 ms |
4.08% |
〰️ |
models tree 50k categories > validate changed visibility (P95 of main thread blocks) |
27 ms |
21 ms |
28.57% |
〰️ |
models tree 50k 3D elements > collect nodes |
47029 ms |
46818 ms |
0.45% |
〰️ |
models tree 50k 3D elements > collect nodes (P95 of main thread blocks) |
76 ms |
61 ms |
24.59% |
〰️ |
models tree 50k 3D elements > validate initial visibility |
1510 ms |
1363 ms |
10.79% |
🚨 |
models tree 50k 3D elements > validate initial visibility (P95 of main thread blocks) |
31 ms |
0 ms |
3100% |
〰️ |
models tree 50k 3D elements > change model visibility |
108 ms |
111 ms |
-2.70% |
〰️ |
models tree 50k 3D elements > change model visibility (P95 of main thread blocks) |
0 ms |
0 ms |
0% |
🟰 |
models tree 50k 3D elements > validate changed model visibility |
2107 ms |
2079 ms |
1.35% |
〰️ |
models tree 50k 3D elements > validate changed model visibility (P95 of main thread blocks) |
0 ms |
0 ms |
0% |
🟰 |
models tree 50k 3D elements > change category node visibility |
455 ms |
495 ms |
-8.08% |
〰️ |
models tree 50k 3D elements > change category node visibility (P95 of main thread blocks) |
41 ms |
57 ms |
-28.07% |
〰️ |
models tree 50k 3D elements > validate changed category visibility |
1510 ms |
1444 ms |
4.57% |
〰️ |
models tree 50k 3D elements > validate changed category visibility (P95 of main thread blocks) |
0 ms |
0 ms |
0% |
🟰 |
models tree 50k 3D elements > validate per-model category override |
1463 ms |
1396 ms |
4.80% |
〰️ |
models tree 50k 3D elements > validate per-model category override (P95 of main thread blocks) |
0 ms |
0 ms |
0% |
🟰 |
models tree 50k 3D elements > change element visibility |
39 ms |
40 ms |
-2.50% |
〰️ |
models tree 50k 3D elements > change element visibility (P95 of main thread blocks) |
0 ms |
0 ms |
0% |
🟰 |
models tree 50k 3D elements > validate changed element visibility |
2141 ms |
2195 ms |
-2.46% |
〰️ |
models tree 50k 3D elements > validate changed element visibility (P95 of main thread blocks) |
57 ms |
53 ms |
7.55% |
〰️ |
models tree 50k 3D child elements with different categories > collect nodes |
65768 ms |
65075 ms |
1.06% |
〰️ |
models tree 50k 3D child elements with different categories > collect nodes (P95 of main thread blocks) |
75 ms |
61 ms |
22.95% |
〰️ |
models tree 50k 3D child elements with different categories > validate initial visibility |
2049 ms |
1937 ms |
5.78% |
〰️ |
models tree 50k 3D child elements with different categories > validate initial visibility (P95 of main thread blocks) |
0 ms |
0 ms |
0% |
🟰 |
models tree 50k 3D child elements with different categories > change visibility |
52 ms |
51 ms |
1.96% |
〰️ |
models tree 50k 3D child elements with different categories > change visibility (P95 of main thread blocks) |
0 ms |
0 ms |
0% |
🟰 |
models tree 50k 3D child elements with different categories > validate changed visibility |
5785 ms |
5658 ms |
2.24% |
〰️ |
models tree 50k 3D child elements with different categories > validate changed visibility (P95 of main thread blocks) |
61 ms |
95 ms |
-35.79% |
〰️ |
categories tree 50k subCategories search > get search paths |
1694 ms |
1733 ms |
-2.25% |
〰️ |
categories tree 50k subCategories search > get search paths (P95 of main thread blocks) |
64 ms |
47 ms |
36.17% |
〰️ |
categories tree 50k subCategories search > load hierarchy from search paths |
5827 ms |
5853 ms |
-0.44% |
〰️ |
categories tree 50k subCategories search > load hierarchy from search paths (P95 of main thread blocks) |
62 ms |
60 ms |
3.33% |
〰️ |
categories tree 50k subCategories > collect nodes |
6267 ms |
6242 ms |
0.40% |
〰️ |
categories tree 50k subCategories > collect nodes (P95 of main thread blocks) |
46 ms |
51 ms |
-9.80% |
〰️ |
categories tree 50k subCategories > validate initial visibility |
1177 ms |
1183 ms |
-0.51% |
〰️ |
categories tree 50k subCategories > validate initial visibility (P95 of main thread blocks) |
26 ms |
28 ms |
-7.14% |
〰️ |
categories tree 50k subCategories > change visibility |
340 ms |
353 ms |
-3.68% |
〰️ |
categories tree 50k subCategories > change visibility (P95 of main thread blocks) |
23 ms |
26 ms |
-11.54% |
〰️ |
categories tree 50k subCategories > validate changed visibility |
1127 ms |
1076 ms |
4.74% |
〰️ |
categories tree 50k subCategories > validate changed visibility (P95 of main thread blocks) |
29 ms |
31 ms |
-6.45% |
〰️ |
categories tree 50k categories > collect nodes |
2716 ms |
2722 ms |
-0.22% |
〰️ |
categories tree 50k categories > collect nodes (P95 of main thread blocks) |
48 ms |
47 ms |
2.13% |
〰️ |
categories tree 50k categories > validate initial visibility |
4373 ms |
3848 ms |
13.64% |
🚨 |
categories tree 50k categories > validate initial visibility (P95 of main thread blocks) |
115 ms |
69 ms |
66.67% |
〰️ |
categories tree 50k categories > change visibility |
791 ms |
751 ms |
5.33% |
〰️ |
categories tree 50k categories > change visibility (P95 of main thread blocks) |
58 ms |
56 ms |
3.57% |
〰️ |
categories tree 50k categories > validate changed visibility |
4000 ms |
3667 ms |
9.08% |
〰️ |
categories tree 50k categories > validate changed visibility (P95 of main thread blocks) |
33 ms |
33 ms |
0% |
🟰 |
classifications tree 50k classifications search > get search paths |
2249 ms |
2213 ms |
1.63% |
〰️ |
classifications tree 50k classifications search > get search paths (P95 of main thread blocks) |
124 ms |
138 ms |
-10.14% |
〰️ |
classifications tree 50k classifications search > load hierarchy from search paths |
63935 ms |
64889 ms |
-1.47% |
〰️ |
classifications tree 50k classifications search > load hierarchy from search paths (P95 of main thread blocks) |
30 ms |
30 ms |
0% |
🟰 |
classifications tree 50k classifications > collect nodes |
33409 ms |
34628 ms |
-3.52% |
〰️ |
classifications tree 50k classifications > collect nodes (P95 of main thread blocks) |
67 ms |
42 ms |
59.52% |
〰️ |
classifications tree 50k classifications > validate initial visibility |
3209 ms |
3099 ms |
3.55% |
〰️ |
classifications tree 50k classifications > validate initial visibility (P95 of main thread blocks) |
63 ms |
60 ms |
5% |
〰️ |
classifications tree 50k classifications > change visibility |
163 ms |
161 ms |
1.24% |
〰️ |
classifications tree 50k classifications > change visibility (P95 of main thread blocks) |
25 ms |
24 ms |
4.17% |
〰️ |
classifications tree 50k classifications > validate changed visibility |
3451 ms |
3323 ms |
3.85% |
〰️ |
classifications tree 50k classifications > validate changed visibility (P95 of main thread blocks) |
75 ms |
64 ms |
17.19% |
〰️ |
This comment was automatically generated by workflow using github-action-benchmark.
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.
presentation-performance-tests, noticed thatModels tree initial loadtests were slower on v4 compared to v3. This was happening because we were executing additional query, to cache data about sub-models (this data is not needed for initial load, but is needed for visibility handling). Decided to adjust the caching behaviour to not execute this query on initial load. - Adjusted modeled elements cache to useIdSetfor better performance.InVirtualSetusage - even though queries are a bit more complex, the performance is better.hasChildrenvalue.