-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Add performance benchmark for the CA RunOnce control loop #9237
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
Open
Choraden
wants to merge
9
commits into
kubernetes:master
Choose a base branch
from
Choraden:run_once_bench_v2
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+540
−14
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
95787cd
Generalize MustCreateManager to support testing.TB
Choraden 5f74be6
Introduce performance benchmarking for the RunOnce control loop
Choraden 6bd19d7
Add CPU profiling support to RunOnce benchmarks
Choraden 75f49a1
Disable predicate parallelism in benchmarks for stability and reprodu…
Choraden dec0f3f
Stabilize benchmark results by disabling GC
Choraden 2c80c57
Silence benchmark logs to avoid output noise
Choraden d7eac11
Add no-op event recorder to benchmarks to avoid dropping events
Choraden f6e089e
Use fast scale-up in RunOnce benchmarks to reduce CPU noise
Choraden 2c85ed6
Use fast tainting reactors in RunOnce benchmark
Choraden File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AutoscalingKubeClientsis normally built based on theKubeClientandInformerFactory, both of which are already injectable (and as far I can see they need to be injected). Why do we need to makeAutoscalingKubeClientsdirectly injectable too? It seems like we'd always want to have theAutoscalingKubeClientsfield in sync with the other two. Otherwise we'd get different behavior for components that use the client/informers directly than for components that useAutoscalingKubeClients.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah okay, after reading some more I see why - we want to change the log recorder. Could you add a warning to the method comment that this is not needed for most use-cases, and if you're using it it has to be in sync with the objects provided in
WithKubeClient()andWithInformerFactory()?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a comment to the method.