-
Notifications
You must be signed in to change notification settings - Fork 4.6k
[v2] Add more granular metrics to benchmarking framework: plugin-load time, static imports time, client creation time. #10292
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
aemous
wants to merge
9
commits into
aws:v2
Choose a base branch
from
aemous:plugin-load-time-perf
base: v2
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.
+148
−24
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
d30c8bf
Update scripts for benchmarking to comput plugin-imports
aemous 6a7b2e2
Emit the load-plugins event.
aemous df1153d
some fixes
aemous e9d63c4
Add mock credentials to all benchmark stubs.
aemous 513f611
Fix stubs template.
aemous f02244c
Update debug dir parameter so it supports relative paths.
aemous d385117
Finalize changes.
aemous c6684d5
Add client creation time.
aemous 647b19c
Remove debug print statement.
aemous 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
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.
What are we measuring here exactly?
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.
This is the time to import all modules used by the AWS CLI at runtime. It's similar to the granular benchmarking that you previously implemented where you measured "Python Library Imports."
It let's us measure the total amount of time spent during the AWS CLI runtime (i.e. after interpreter started) that is spent importing modules, before we start running any initialization code.
It is largely prompted by our previous observation that
prompt_toolkittook up a significant fraction of runtime. It embeds the philosophy "if module imports previously took up too much runtime, we should monitor the metric over time so we don't lose track of it, to prevent future regressions."During my dev builds, we're observing 119 ms of time taken during static imports. This starts off as a baseline, that we track changes in over time.
It also automatically implements the measure that we previously did ad-hoc. When we benchmarked it ad-hoc we found useful results (particularly hefty imports). So in my view, if it was useful once, then it should be useful to track in the long term.
"What we expect it to catch, how we would react"
For the most part, it should catch large regressions. If we notice this number change significantly, it would prompt us to investigate the regression, and consider refactoring code (e.g. lazy initialization). Overall, this should help us reduce the time between (A) unintentionally increasing command execution time due to a new hefty import and (B) allocating time to investigate and mitigate the regression.