-
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
base: master
Are you sure you want to change the base?
Conversation
This is a preparation for adding unit-started event.
Instead of all unnecesary field, just track kind and in log. Detailed info can be retrieved from `cargo metadata` or other tools
Reduce log size by using an integer index instead of full metadata. The index is assigned deterministically at initialization.
New events * UnitRmetaFinished * UnitSectionStarted * UnitSectionFinished So that no need to track rmeta and section timings in unit-finished
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=json emits 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 like{ "event": "start", /* Marks the "start" or "end" of the compilation section */ "name": "link", /* The name of the compilation section */ // Opaque timestamp when the message was emitted, in microseconds // The timestamp is currently relative to the beginning of the compilation session "time": 12345 }
I don't feel like there is anything we can directly use. timing-info Cargo message is half-broken. compiler-artifact has 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 on section_timing's time field, 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.
What does this PR try to resolve?
Convert timing-info log messages from a single aggregated message per unit
to an event-based model that captures the build timings.
New events introduced:
unit-started- compilation begins (full metadata + index)unit-rmeta-finished-.rmetaartifact generated (with unlocked units)unit-section-{started,finished}- compilation phases (with section name)unit-finished- completion (with unlocked units)unit-startedChanges in old events:
timing-info— split to fiveunit-*eventsrebuild— thetargetfield now use a custom struct to show{ name, kind }, which should be good enough to distinguish different units.Part of #15844.
How to test and review this PR?
Test diffs should reflect the change.
A new test has bee added to showcase
-Zsection-timings(apparently we didn't have any test before).