Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

frontend: Improve Map performance #2538

Merged
merged 6 commits into from
Nov 8, 2024
Merged

frontend: Improve Map performance #2538

merged 6 commits into from
Nov 8, 2024

Conversation

sniok
Copy link
Contributor

@sniok sniok commented Nov 7, 2024

This fixes a stack overflow crash #2534 and improves grouping and filtering performance

There's still a bunch of stuff that can be optimized so this is only first step

Testing done:

  • Tested on KWOK cluster with 20,000 pods

After fixes:
Map loading time: 5s
Filter by error: <1s
Group by node: <1s

Before:
Map loading time: ~3.5min
Filter by error: 50s
Group by node: 9s

@sniok sniok requested a review from a team November 7, 2024 14:31
@sniok sniok marked this pull request as ready for review November 7, 2024 14:31
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Nov 7, 2024
Copy link
Contributor

@skoeva skoeva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huge perf improvement, thanks for this!

@illume
Copy link
Collaborator

illume commented Nov 7, 2024

Very nice.

Did you look into how it reacts with a fairly high frequency of change? I mean with pods/events being added, rather than with a static set of resources.
(If not, probably something to look at in another PR.)

Copy link
Collaborator

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 7, 2024
@sniok
Copy link
Contributor Author

sniok commented Nov 8, 2024

Did you look into how it reacts with a fairly high frequency of change? I mean with pods/events being added, rather than with a static set of resources. (If not, probably something to look at in another PR.)

Yeah it struggles a bit with frequent changes, but that is another issue

@illume
Copy link
Collaborator

illume commented Nov 8, 2024

Doh. I think I broke the test plugins CI job with the release of the new headlamp-k8s/eslint-config. It is trying to upgrade to the "latest" headlamp-k8s/eslint-config, where it should be getting the version related to that version of headlamp-plugin.

So you can ignore that test plugins job failure in this PR.

@sniok
Copy link
Contributor Author

sniok commented Nov 8, 2024

Doh. I think I broke the test plugins CI job with the release of the new headlamp-k8s/eslint-config. It is trying to upgrade to the "latest" headlamp-k8s/eslint-config, where it should be getting the version related to that version of headlamp-plugin.

So you can ignore that test plugins job failure in this PR.

Got it, thanks for the explanation

Copy link
Collaborator

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@illume illume merged commit 5113bf1 into main Nov 8, 2024
17 of 18 checks passed
@illume illume deleted the map-perf branch November 8, 2024 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer resourceMap size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants