-
Notifications
You must be signed in to change notification settings - Fork 2.7k
fix(log): break timing-info message to multiple #16303
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
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
12958ef
docs(timings): semantic line-break (for just one paragraph)
weihanglo 6562d03
refactor(timings): clarify unblocked units
weihanglo b42bec2
fix(log): rename timing-info to unit-finished
weihanglo ea5850f
fix(log): log target name and kind
weihanglo 78582e7
feat(log): add unit-started event
weihanglo 5f839d3
refactor(timings): dont init anything if not enabled
weihanglo 239697a
fix(log): replace unit metadata with index for `unit-*` event
weihanglo 3c23e2d
feat(log): track unblocked units in unit-started event
weihanglo a70ec7f
fix(log): recored elapsed time rather than duration
weihanglo 1ce4adb
docs(log): brief description for each log message field
weihanglo 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
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.
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.
Before I dig into this too much, do we need special messages for these or are there existing json messages from cargo we could use if we also log those? If not, are there existing rustc messages that we can translate into cargo messages to handle this?
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 Cargo
--message-format=jsonemits today:build-finished— for the entire build, not at per unit levelbuild-script-exectued— emitted after build script executioncompiler-artifact— emitted when a compiler artifact is produced for each rustc invocation.compiler-message— this is basically rustc diagonstics.timing-info— the unstable aggregated timing info. It is the same as the old timing-info log message before this PR.None of them has any elapsed time information
What rustc has today:
diagnostic— just diagnosticsartifact— Notice when an artifact is written to disk. Contains file name and emit-type.future_incompat— for future incompat warningssection_timing— unstable section timing. Looks likeI don't feel like there is anything we can directly use.
timing-infoCargo message is half-broken.compiler-artifacthas too much of information timing tracking doesn't need (but perhaps we'd like to expand to that in the future). We might not want to rely onsection_timing'stimefield, as it is unclear what "beginning of the compilation" refers to.The current approach gives us a room to iterate on our own at least.
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.
If we view this as a hack and document that we need to consolidate our messages, I'm fine moving forward with this.
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.
Yeah it is not the final version. I'd love to learn your thoughts on long-term consolidating our JSON messages and logging.
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 think we need to flesh out our json messages more
From #7614 (comment)
If we log all json messages, we'll have redundancy with this.
From #t-cargo > build analysis log format @ 💬
I also wonder if we should go with a full inverse, all logged messages are also json messages. Of course, we'll likely need to keep them distinct while the feature is unstable and we work through what we want the schema to be.