-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Add profiler to bootstrap command #143525
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
base: master
Are you sure you want to change the base?
Conversation
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.
Thank you! I don't think that we'll need JSON output for now, I think that the main output should be some lightly formatted text that will be human interpretable.
This comment has been minimized.
This comment has been minimized.
2730ce9
to
aeec9c4
Compare
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.
Gonna try it tomorrow.
The job Click to see the possible cause of the failure (guessed by this bot)
|
@@ -350,6 +351,7 @@ impl<'a> BootstrapCommand { | |||
.map(|(k, v)| (k.to_os_string(), v.map(|val| val.to_os_string()))) | |||
.collect(), | |||
cwd: command.get_current_dir().map(Path::to_path_buf), | |||
short_cmd: command.format_short_cmd(), |
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.
Can you please instead implement FormatShortCmd
on the fingerprint, and then use that implementation in the FomatShortCmd
for the commands? To derive the short cmd from the fingerprint, instead of storing duplicated data in it. It is supposed to be a cache key, so I don't want to include additional human-readable data in it.
@@ -102,3 +102,4 @@ flake.lock | |||
/default.nix | |||
|
|||
# Before adding new lines, see the comment at the top. | |||
*.txt |
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.
I don't think this is needed.
This PR adds command profiling to the bootstrap command. It tracks the total execution time and records cache hits for each command. It also provides the ability to export execution result to a JSON file. Integrating this with Chrome tracing could further enhance observability.
r? @Kobzol