fix(ui): bound how much of a large resource tree is drawn (#10253) - #29125
Open
himeshp wants to merge 12 commits into
Open
fix(ui): bound how much of a large resource tree is drawn (#10253)#29125himeshp wants to merge 12 commits into
himeshp wants to merge 12 commits into
Conversation
✅ Preview Environment deployed on Bunnyshell
See: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
Bundle ReportChanges will increase total bundle size by 7.7kB (0.07%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: argo-cd-ui-array-pushAssets Changed:
|
himeshpanc
added a commit
to himeshpanc/argocd-14k-perf-repro
that referenced
this pull request
Aug 10, 2026
The fixture here is 14k resources wide and one level deep, and every one of
them is a root. That is one shape, and a UI can be badly wrong on others
while looking correct on this one. Two real bugs in the fix for this very
problem were found by review rather than by this reproducer, and neither
could have been reproduced with what was here:
- Code that decides how much of a tree to draw from the number of roots
rather than the number of nodes is indistinguishable from correct on a
fixture where those are the same number. values-deep.yaml gives 181
roots and 331 nodes, where they are not. Deployments at zero replicas
still get a ReplicaSet each, so the depth costs nothing to run.
- A resource with several owners is a single node reached by several
paths. Code that charges work per path rather than per node meets that
for the first time in production. add-shared-owners.sh produces it, and
it is the shape of argoproj/argo-cd#14274. It has to be a script rather
than a template because an ownerReference needs a uid the API server
has not assigned yet.
values-pods.yaml adds a deliberately small real workload, which is the only
way to exercise the UI's compact pod-group path and the only fixture with
genuine health status. Twenty Deployments, not a hundred and fifty: this is
the one that schedules real containers.
The default fixture is unchanged and still renders exactly 14,451
resources; the new counts default to zero.
findings.md records the fix (argoproj/argo-cd#29125) and its measurements,
including a baseline worse than the original: on master with the React
Compiler enabled the tree blocks for over 587s and never becomes
interactive, so the compiler does not address this. It also corrects where
the re-render cost actually sits, which is not where the original profiles
point: once the node count is bounded, dagre.layout() is 28ms and the
dominant cost was sorting every root to select a couple of hundred.
Signed-off-by: himeshpanc <himeshpanc@users.noreply.github.com>
The tree hands every resource to dagre and lays the whole graph out on the main thread during render. The cost is superlinear, so an application with a few thousand resources locks the tab: measured on a 14,451 resource application, the page never became interactive within ten minutes. Draw a bounded number of nodes instead. processNode now reports whether it drew a node, so callers only draw an edge when there is something to draw an edge to, and children are capped per parent so one very wide subtree cannot consume the whole budget. Same application, same build: interactive in about ten seconds. Refs argoproj#10253 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: himeshp <himeshp@users.noreply.github.com>
Bounding the graph is only half of it. Spending the budget in name order fills the view with whichever kind sorts first, so on a 14,451 resource application the user got 200 ConfigMaps and none of the six Deployments they came to look at. And nothing said the other 14,251 existed, which is worse than drawing them slowly: the view looked complete. Rank resources by how much attention they need before spending the budget: degraded and missing first, then progressing, then out of sync, then suspended, then healthy workloads, then the things that expose them, then everything else. A parent inherits the most interesting state beneath it, so a healthy looking Deployment with a failing pod still surfaces. Ordering only changes when the budget actually bites. Then say what was dropped, both for the application as a whole and for any one parent whose children were capped. Same application: the tree now opens on all six Deployments with their ReplicaSets and pods, and a card reading "Showing 151 of 14457 resources". Refs argoproj#10253 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: himeshp <himeshp@users.noreply.github.com>
With one shared budget every kind competes for it, which is why selecting what to draw needed ranking at all: whichever kind sorted first consumed the allowance and the rest rendered nothing. Give the bulk kinds a parent of their own and the contest goes away. The top level holds the workloads, whose hierarchy is the reason this is a graph rather than a list, plus one node per remaining kind carrying that kind's real total. It no longer grows with the size of the application. Each kind previews the few members that most need attention and has its own marker for the rest. Workloads and anything with children are never folded away, and a kind small enough to show outright is left alone. On a 14,451 resource application the top level is now six Deployments, one CustomResourceDefinition and thirteen kind nodes, and the whole tree draws 108 nodes. Refs argoproj#10253 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: himeshp <himeshp@users.noreply.github.com>
Builds on the budget, ranking and kind nodes with the parts that make a bounded view usable rather than merely small. Markers now say what they hide, not just how much: the states behind them are summarised with the tree's own health and sync icons, worst first, so a marker hiding nothing but healthy resources can be left alone. Each marker grows by a fixed step rather than uncapping its parent outright, raising the graph's budget alongside its own so the extra nodes are actually admitted, and stops at a ceiling with an explanation instead of offering a click it cannot honour. A kind node drills into its kind, and a collapsed kind reports the kind's real total rather than the sample it collapsed. The network view had the same silent truncation the tree used to have, and worse: its roots were placed directly, so the budget only ever refused their children and nothing reported the loss. Roots now go through the budget, and edges to children, load balancers and traffic nodes are only drawn once the root they hang off was admitted. The resource filter panel deduplicated names with indexOf, which is quadratic when every name is distinct, and handed the autocomplete every resource name in the application, mounting one hidden DOM node per resource. Deduplicate with a Set, memoise the derived lists, and hand the autocomplete a bounded slice narrowed by what has been typed. The toolbar's expand and collapse now move one level at a time. Collapsing every parent at once left nothing for a second click to do, and it collapsed the application node too, which made the tree skip building its roots and emptied the view. Refs argoproj#10253, argoproj#14274 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: himeshp <himeshp@users.noreply.github.com>
…olds Expanding or collapsing a node re-ranks everything dagre draws, so the node that was clicked could travel a long way: expanding a Deployment moved it 1,502px down the layout and 116px out from under the cursor, which threw the user out of the place they were looking at. Record where the node sat before the toggle and scroll by however far it moved. Both readings come from dagre's own coordinates rather than from the DOM. By the time a layout effect runs React has already written the new inline offsets, but offsetTop still reports the old ones, so measuring the elements compares a new position against a stale one and corrects by nothing. The nodes carry their graph key so the pending anchor can be looked up in the rebuilt graph. Collapsing cannot always be honoured: a node whose subtree is removed from above it rises to the top of the graph and there is no scroll left to give back. The anchor is recorded from a capture-phase handler on the tree container, which runs before the toggle's own handler and so still sees the layout on screen. That also keeps the ref writes in an event handler and out of render, where reading them is a Rules of React violation. A collapsed node also said nothing about what it was hiding, so the "+" was a promise with no size attached. Show the child count beside it. The count it had to show was wrong: rather than counting children it added the running length of the array it was building, so it grew quadratically and only survived because the caller used it as "> 0". Count children. Refs argoproj#10253, argoproj#14274 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: himeshp <himeshp@users.noreply.github.com>
…hing
Ranking the tree by relevance made every re-render pay for ordering the
whole application in order to draw a couple of hundred of it. On a 14,511
resource application a re-render cost 220ms of graph building, of which
120ms was one sort of ~14k roots and most of the rest was sorting a
4,000 member kind by name to take 5 of it. Zoom, pan and each streamed
update all paid it, as a single blocking task.
Keep the best `limit` items in one pass instead. An item that cannot
displace the worst one kept is rejected on a single comparison, which is
what nearly every item does, so the cost tracks the number of resources
rather than the number of resources times their logarithm. The comparator
is unchanged, so what gets drawn is the same as before.
Partitioning now happens before any ordering, which it can because
partitioning never needed sorted input. That also fixes what the change
would otherwise have broken: a kind node counts every member it stands
for, not only those that reached the top of a sort. Ordering yields a
prefix rather than the whole set, so the card's total comes from the set
that went in.
A clustered kind only ever holds childless roots, so a member's subtree
relevance is its own. Using that directly avoids the memoised walk, which
would build a key string per member to discover it has no children.
Measured on the same 14,511 resource application, React Compiler enabled:
graph build 220ms -> 45ms
of which sort 120ms -> 15ms
dagre layout 28ms (unchanged)
blocking task 317ms -> 157ms
Refs argoproj#10253, argoproj#14274
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: himeshp <himeshp@users.noreply.github.com>
…rops Review of the bounded tree found several ways it could withhold resources without saying so. The worst was self-contradictory: the summary card tells the user to search for the resources it hid, but the budget was spent before the filter was applied, so a filter could only ever narrow what had already been drawn. Searching for one of 4,000 ConfigMaps by name found nothing. Ranking now puts anything the filter reaches ahead of everything it does not, at the top level, among a parent's children and inside a clustered kind, so the matches survive the budget. The network view ranked its roots by name alone and never consulted the filter at all; it now orders them the same way the tree does. A child the global budget refused was dropped silently. Only children refused by the per-parent cap were counted, so a subtree could lose descendants with no overflow marker and no way to ask for them back, while the count that recorded the loss was never read by anything. Those children are now counted like any other hidden child, and the unread counter is gone. The toolbar's level control describes an ownership hierarchy, but the network view draws its parents from networkingInfo, where those depths collapse the wrong nodes. Restore the all-or-nothing behaviour there, which is what that view had before. Collapsing also enumerated every node rather than only the parents that can actually be collapsed, and found the deepest level by spreading one argument per node, which overflows the call stack on a large application. Remaining fixes: a kind node counted only the members clustered beneath it while drilling into it showed every root of that kind, so it now reports the full tally; the clustering threshold is decided from the size of the application rather than the running budget, so expanding a marker can no longer reshape the whole top level mid-session; the drill-in and the budget are keyed to one view of one application instead of leaking across both; each service below an ingress carries its own traffic colour into its own subtree rather than the root's, which for an external root was undefined; and asking to show one more level when nothing is hidden no longer rebuilds the graph to no effect. The autocomplete prefilter compared label substrings, but argo-ui matches abbreviations, accepts globs, and falls back to the whole list when nothing matches. The prefilter defeated all three, so "svc" no longer found Service. It now compares abbreviations too, leaves the list alone for a glob, and falls back to the head of the list rather than nothing. Tests cover the selection, the filter-aware subtree matching and the relevance walk, including the cycle guards. Refs argoproj#10253, argoproj#14274 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: himeshp <himeshp@users.noreply.github.com>
A second review of the bounded tree found one design flaw behind most of its remaining holes: a single counter was deciding the shape of the top level, rationing the roots, and rationing every overflow marker, so those three uses stole from each other. Ordering was decided from the number of roots, but the budget is spent on nodes. An application of 150 deployments with their replica sets and pods has fewer roots than the cap and far more nodes, so its roots stayed in name order while traversal still truncated them: later chains were dropped unranked and a search matching one of them built nothing, leaving an apparently empty tree. Clustering, which really is about how crowded the top level is, is now decided separately from ordering, and a filter always forces ordering. Roots now draw against an allowance of their own. Raising the budget for one marker has to reach that marker, but while roots shared the budget they spent the increment before the marker was processed, so a clustered kind starved by the initial cap stayed empty however many times it was clicked. The card that asks for more roots raises their allowance too. A node reached through several parents was charged to the budget once per path, and its subtree walked again each time, though the graph holds it once. Applications shaped like argoproj#14274, where hundreds of resources share three parents, could exhaust the budget at a third of their real size. The network view selected its external and internal roots separately, each against the whole allowance, and placed the external ones first, so enough of them starved an internal root the search had matched. They are now ranked as one set against the budget they share, and the roots it turns away get an overflow control, which this view never had. Compact mode moves a parent's pods onto its podGroup, out of the child map, so subtree matching could not see them: searching for a hidden pod by name found nothing, because the budget discarded its parent chain before filterGraph looked at the group. Matching now checks grouped pods the same way filterGraph does. Compact mode is the default, so this was the common path rather than an edge case. Overflow controls are told whether the graph is full once traversal has finished and the real spend is known. Deciding it from the cap alone disabled expansion with capacity to spare, because a parent's allowance grows more slowly than the cap does. Tests cover the two shapes that let these through, both invisible on a wide fixture: an application deep enough to exhaust the budget with few roots, and pods hidden inside a compacted parent. Refs argoproj#10253, argoproj#14274 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: himeshp <himeshp@users.noreply.github.com>
…s drawn Two mistakes in the previous commit, both of it taking a value to mean something it does not. nodeFilter is a required prop and the caller always supplies a closure, so its presence says nothing about whether the user has filtered anything. Reading it as "a filter is active" made that condition always true, which forced relevance ordering onto every application including the small ones that fit comfortably inside the budget. Those trees lost the order they have always had, which is the opposite of what the ordering was gated on in the first place, and every node paid for a subtree walk whose predicate could only return true. The filter list is the signal. The network view's new overflow control counted the roots the allowance admitted rather than the roots that were drawn. An admitted root can still be refused once an earlier root's descendants have spent the budget, so a deep graph could report two hundred roots shown when only a handful reached the graph, and a graph whose roots all fitted reported nothing at all however much was truncated beneath them. Count what processNode accepted, which is what the tree view already did. Refs argoproj#10253, argoproj#14274 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: himeshp <himeshp@users.noreply.github.com>
…d tests The last two review rounds each found a bug introduced by the round before it. That is a property of where this logic lives rather than of any one mistake: the budget, each parent's allowance, the defaults, the step and the ceiling were juggled inline across four hundred lines of graph building, they could only be exercised by loading a real application, and the tests that existed took an already-derived boolean and so could not catch a caller deriving it wrongly. Move them into a module of their own where the interactions can be tested directly. That surfaced the flaw this round reported. An overflow control passed the count of what was drawn as the baseline for asking for more. On a deep graph, where descendants spend the budget before later roots are reached, far fewer are drawn than the allowance admitted, so the first click set the allowance to the drawn count plus a step: asking for more revealed less. Controls now carry the allowance that was in effect rather than the count displayed, which cannot shrink. The two expansion paths that had drifted apart are now one. The card and the markers both went through their own handler with their own idea of the baseline, which is how only one of them acquired the bug. Tests cover the arithmetic and the two signals a caller has to derive: an unfiltered application keeps its order however the filter callback is supplied, and one click on a deep graph raises the admission window rather than lowering it. Both are the shapes the previous rounds got wrong. Refs argoproj#10253, argoproj#14274 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: himeshp <himeshp@users.noreply.github.com>
himeshp
force-pushed
the
perf/bounded-resource-tree
branch
from
August 11, 2026 04:26
6e3c015 to
79fb20a
Compare
himeshp
marked this pull request as ready for review
August 11, 2026 07:06
PR Summary by QodoBound Application resource tree rendering with relevance ranking and overflow markers
AI Description
Diagram
High-Level Assessment
Files changed (7)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+1156
to
+1159
| <a className={`group-nodes-button`} onClick={() => expandAll()} title='Show one more level of child nodes'> | ||
| <i className='fa fa-plus fa-fw' /> | ||
| </a> | ||
| <a className={`group-nodes-button`} onClick={() => collapseAll()} title='Collapse all child nodes of all parent nodes'> | ||
| <a className={`group-nodes-button`} onClick={() => collapseAll()} title='Hide the deepest level of child nodes'> |
There was a problem hiding this comment.
1. Resource tree change undocumented 📘 Rule violation ⚙ Maintainability
This PR changes default Application resource tree behavior (bounded node budget, new overflow/expand semantics), but no corresponding updates were made to docs/ to explain the new UX and limits.
Agent Prompt
## Issue description
The Application resource tree now draws a bounded subset of nodes (with new expand/collapse semantics and overflow markers), but the user-facing documentation in `docs/` was not updated to describe the new behavior, defaults (e.g., `DEFAULT_VISIBLE_CAP`), and how users can reveal hidden resources.
## Issue Context
This is a user-visible behavior change in the UI that can affect how operators interpret the resource tree for large Applications. Documentation should explain what the UI will show by default, how overflow markers work, and how search/filter interacts with the budget.
## Fix Focus Areas
- ui/src/app/applications/components/application-details/application-details.tsx[1156-1160]
- ui/src/app/applications/components/application-resource-tree/application-resource-tree-budget.ts[8-18]
- ui/src/app/applications/components/application-resource-tree/application-resource-tree.tsx[118-136]
- docs/user-guide/resources-view.md[1-40]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The network view's collapse control walked tree.nodes only, so orphaned resources never collapsed. The tree draws them whenever orphaned resources are enabled, and the depths gathered a few lines above for the tree view already include them, so a control that claims to collapse everything was reaching a smaller set than the view it acts on. This is a regression rather than a rough edge in new behaviour: the all-or-nothing collapse this restored was built from tree.nodes concatenated with orphanedNodes, and the concatenation was dropped in the restoring. Collapsing a uid that is not drawn costs nothing, so this does not consult the orphaned-resources preference, which keeps it the same shape as the depth gathering above it. Verified by type-checking and by matching the behaviour it restores. Not verified in a browser: the condition needs orphaned resources that also carry networkingInfo, which the reproduction Application does not have, and the control lives inside a DataLoader render callback where a unit test cannot reach it. Refs argoproj#10253 Signed-off-by: himeshp <himeshp@users.noreply.github.com>
The tree now draws a bounded part of a large Application, which changes what users see by default and was documented nowhere. The existing resources-view page is about the aggregated /resources table and points readers at the application tree for owned resources, so this is a page of its own rather than a section there. Written in terms of what is observable -- the summary card, the overflow markers and what they say they hold, the kind nodes, and that a search reaches resources that were not drawn -- and deliberately not in terms of the specific limits, which are tuning rather than interface. A note says as much, so nobody builds on the numbers. Refs argoproj#10253 Signed-off-by: himeshp <himeshp@users.noreply.github.com>
Contributor
Author
|
Pushed two changes from review feedback. Network view collapse was missing orphaned resources. The collapse control walked I have not verified that one in a browser. It needs orphaned resources that also carry Docs: new page 334 tests across 26 suites, |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #29125 +/- ##
==========================================
- Coverage 65.61% 65.57% -0.05%
==========================================
Files 427 427
Lines 60556 60564 +8
==========================================
- Hits 39733 39713 -20
- Misses 17192 17214 +22
- Partials 3631 3637 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
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.
Draft, opened for discussion before polish. The behaviour here changes what the resource tree shows by default, so I would rather agree the approach than arrive with it settled. Specific questions for reviewers are at the bottom.
The problem
On an Application with ~14k resources the tree view never becomes usable. Measured on
masterwith the React Compiler enabled, the main thread stays blocked for over 587 seconds and the page never becomes interactive — it is not slow, it is unavailable.The cost is client-side: the whole tree is built and handed to
dagre.layout()on the main thread, and layout cost grows faster than the resource count. Nothing in the UI decides that an Application is too large to draw in the normal way, which is what #10253 asks for and roughly what a maintainer suggested there — that at some point the UI has to decide a tree is too large to display normally. #14274 profiles the samelayout()call on a much smaller Application whose resources share several parents.What this does
Draws a bounded, ranked part of the tree and says clearly what it left out.
perf-ui-pr-34s.mp4
Measurements
Same Application (14k resources), React Compiler enabled, before and after:
Sampled three times at 250ms granularity; the "before" figures come from the same probe, which stops
waiting at 587s rather than because anything happened. Both sides are measured on a webpack dev build, so
they are comparable to each other, and a production bundle should do better than the "after" column.
The re-render work came down by replacing a full sort of every root with a bounded selection: ordering ~14k roots to draw 200 of them cost 120ms on every zoom, pan and streamed update.
What this does not do
Questions for reviewers
argocd-cm?Happy to split this into smaller PRs if that is easier to review — the budget, the ranking and the clustering are separable.
Reproduction
Everything above is independently checkable: https://github.com/himeshpanc/argocd-14k-perf-repro
One
kubectl applyagainst a cluster already running Argo CD creates a single Application of whatever sizeand shape you want. It ships a Helm chart that generates the resources, served from an in-cluster git
daemon, so it exercises the real path (repo-server → controller → resource tree → UI) rather than a fixture
loaded into the browser.
values-deep.yamlvalues-pods.yamlscripts/add-shared-owners.shThe last two fixtures are there because they found bugs in this PR. The default fixture is 14k resources
wide and one level deep, where the number of roots and the number of nodes are the same, so code that
confuses the two looks correct; and a resource with several owners is a single node reached by several
paths, which code that charges work per path gets wrong. Both were real defects here, found by review
rather than by the reproducer, which is why it can now produce those shapes.
The repo also carries the Chrome CPU profiles from the original investigation — those load straight into
DevTools → Performance with no cluster needed — and a writeup of the method, including why the main thread
being blocked means the measurement has to come from outside the browser.
Related work
Prior art on the same bottleneck, so this can be placed against it rather than duplicating it:
layout()call, on an Application whose resources share several parentsresource.exclusionsmitigation people currently reach forIf the canvas rewrite in #19906 is the preferred direction, I would rather know that now than after
polishing this.
Addresses #10253. Related to #14274.
Checklist:
docs/proposals/covering the conventions, rather than settling them in a code review.tsc --noEmitclean, ESLint clean includingeslint-plugin-react-hooksv7. CI on this PR is the real check.