Skip to content

Commit 3d20f19

Browse files
authored
fix(log): break timing-info message to multiple (#16303)
### 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` - `.rmeta` artifact generated (with unlocked units) - `unit-section-{started,finished}` - compilation phases (with section name) - `unit-finished` - completion (with unlocked units) - All events use elapsed time instead of duration - All events reference units by deterministic index from `unit-started` Changes in old events: * `timing-info` — split to five `unit-*` events * `rebuild` — the `target` field 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).
2 parents 23f5b2b + 1ce4adb commit 3d20f19

File tree

8 files changed

+462
-84
lines changed

8 files changed

+462
-84
lines changed

src/cargo/core/compiler/fingerprint/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ pub fn prepare_target(
454454
if !dirty_reason.is_fresh_build() {
455455
logger.log(LogMessage::Rebuild {
456456
package_id: unit.pkg.package_id().to_spec(),
457-
target: unit.target.clone(),
457+
target: (&unit.target).into(),
458458
mode: unit.mode,
459459
cause: dirty_reason.clone(),
460460
});

src/cargo/core/compiler/job_queue/mod.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,8 @@ impl<'gctx> DrainState<'gctx> {
606606
.gctx
607607
.shell()
608608
.verbose(|c| c.status("Running", &cmd))?;
609-
self.timings.unit_start(id, self.active[&id].clone());
609+
self.timings
610+
.unit_start(build_runner, id, self.active[&id].clone());
610611
}
611612
Message::Stdout(out) => {
612613
writeln!(build_runner.bcx.gctx.shell().out(), "{}", out)?;
@@ -712,7 +713,7 @@ impl<'gctx> DrainState<'gctx> {
712713
self.tokens.push(token);
713714
}
714715
Message::SectionTiming(id, section) => {
715-
self.timings.unit_section_timing(id, &section);
716+
self.timings.unit_section_timing(build_runner, id, &section);
716717
}
717718
}
718719

@@ -1122,10 +1123,12 @@ impl<'gctx> DrainState<'gctx> {
11221123
unit.show_warnings(build_runner.bcx.gctx),
11231124
)?;
11241125
}
1125-
let unlocked = self.queue.finish(unit, &artifact);
1126+
let unblocked = self.queue.finish(unit, &artifact);
11261127
match artifact {
1127-
Artifact::All => self.timings.unit_finished(build_runner, id, unlocked),
1128-
Artifact::Metadata => self.timings.unit_rmeta_finished(id, unlocked),
1128+
Artifact::All => self.timings.unit_finished(build_runner, id, unblocked),
1129+
Artifact::Metadata => self
1130+
.timings
1131+
.unit_rmeta_finished(build_runner, id, unblocked),
11291132
}
11301133
Ok(())
11311134
}

src/cargo/core/compiler/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1108,7 +1108,8 @@ fn add_allow_features(build_runner: &BuildRunner<'_, '_>, cmd: &mut ProcessBuild
11081108
/// [`--error-format`]: https://doc.rust-lang.org/nightly/rustc/command-line-arguments.html#--error-format-control-how-errors-are-produced
11091109
fn add_error_format_and_color(build_runner: &BuildRunner<'_, '_>, cmd: &mut ProcessBuilder) {
11101110
let enable_timings = build_runner.bcx.gctx.cli_unstable().section_timings
1111-
&& !build_runner.bcx.build_config.timing_outputs.is_empty();
1111+
&& (!build_runner.bcx.build_config.timing_outputs.is_empty()
1112+
|| build_runner.bcx.logger.is_some());
11121113
if enable_timings {
11131114
cmd.arg("-Zunstable-options");
11141115
}

src/cargo/core/compiler/timings/mod.rs

Lines changed: 120 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use crate::util::{CargoResult, GlobalContext};
1717

1818
use cargo_util::paths;
1919
use indexmap::IndexMap;
20+
use itertools::Itertools as _;
2021
use std::collections::HashMap;
2122
use std::io::BufWriter;
2223
use std::time::{Duration, Instant};
@@ -51,6 +52,11 @@ pub struct Timings<'gctx> {
5152
total_fresh: u32,
5253
/// Total number of dirty units.
5354
total_dirty: u32,
55+
/// A map from unit to index.
56+
///
57+
/// This for saving log size.
58+
/// Only the unit-started event needs to hold the entire unit information.
59+
unit_to_index: HashMap<Unit, u64>,
5460
/// Time tracking for each individual unit.
5561
unit_times: Vec<UnitTime>,
5662
/// Units that are in the process of being built.
@@ -89,10 +95,10 @@ struct UnitTime {
8995
/// The time when the `.rmeta` file was generated, an offset in seconds
9096
/// from `start`.
9197
rmeta_time: Option<f64>,
92-
/// Reverse deps that are freed to run after this unit finished.
93-
unlocked_units: Vec<Unit>,
94-
/// Same as `unlocked_units`, but unlocked by rmeta.
95-
unlocked_rmeta_units: Vec<Unit>,
98+
/// Reverse deps that are unblocked and ready to run after this unit finishes.
99+
unblocked_units: Vec<Unit>,
100+
/// Same as `unblocked_units`, but unblocked by rmeta.
101+
unblocked_rmeta_units: Vec<Unit>,
96102
/// Individual compilation section durations, gathered from `--json=timings`.
97103
///
98104
/// IndexMap is used to keep original insertion order, we want to be able to tell which
@@ -127,18 +133,41 @@ struct UnitData {
127133
start: f64,
128134
duration: f64,
129135
rmeta_time: Option<f64>,
130-
unlocked_units: Vec<usize>,
131-
unlocked_rmeta_units: Vec<usize>,
136+
unblocked_units: Vec<usize>,
137+
unblocked_rmeta_units: Vec<usize>,
132138
sections: Option<Vec<(String, report::SectionData)>>,
133139
}
134140

135141
impl<'gctx> Timings<'gctx> {
136142
pub fn new(bcx: &BuildContext<'_, 'gctx>, root_units: &[Unit]) -> Timings<'gctx> {
143+
let start = bcx.gctx.creation_time();
137144
let has_report = |what| bcx.build_config.timing_outputs.contains(&what);
138145
let report_html = has_report(TimingOutput::Html);
139146
let report_json = has_report(TimingOutput::Json);
140147
let enabled = report_html | report_json | bcx.logger.is_some();
141148

149+
if !enabled {
150+
return Timings {
151+
gctx: bcx.gctx,
152+
enabled,
153+
report_html,
154+
report_json,
155+
start,
156+
start_str: String::new(),
157+
root_targets: Vec::new(),
158+
profile: String::new(),
159+
total_fresh: 0,
160+
total_dirty: 0,
161+
unit_to_index: HashMap::new(),
162+
unit_times: Vec::new(),
163+
active: HashMap::new(),
164+
concurrency: Vec::new(),
165+
last_cpu_state: None,
166+
last_cpu_recording: Instant::now(),
167+
cpu_usage: Vec::new(),
168+
};
169+
}
170+
142171
let mut root_map: HashMap<PackageId, Vec<String>> = HashMap::new();
143172
for unit in root_units {
144173
let target_desc = unit.target.description_named();
@@ -156,29 +185,33 @@ impl<'gctx> Timings<'gctx> {
156185
.collect();
157186
let start_str = jiff::Timestamp::now().to_string();
158187
let profile = bcx.build_config.requested_profile.to_string();
159-
let last_cpu_state = if enabled {
160-
match State::current() {
161-
Ok(state) => Some(state),
162-
Err(e) => {
163-
tracing::info!("failed to get CPU state, CPU tracking disabled: {:?}", e);
164-
None
165-
}
188+
let last_cpu_state = match State::current() {
189+
Ok(state) => Some(state),
190+
Err(e) => {
191+
tracing::info!("failed to get CPU state, CPU tracking disabled: {:?}", e);
192+
None
166193
}
167-
} else {
168-
None
169194
};
195+
let unit_to_index = bcx
196+
.unit_graph
197+
.keys()
198+
.sorted()
199+
.enumerate()
200+
.map(|(i, unit)| (unit.clone(), i as u64))
201+
.collect();
170202

171203
Timings {
172204
gctx: bcx.gctx,
173205
enabled,
174206
report_html,
175207
report_json,
176-
start: bcx.gctx.creation_time(),
208+
start,
177209
start_str,
178210
root_targets,
179211
profile,
180212
total_fresh: 0,
181213
total_dirty: 0,
214+
unit_to_index,
182215
unit_times: Vec::new(),
183216
active: HashMap::new(),
184217
concurrency: Vec::new(),
@@ -189,7 +222,7 @@ impl<'gctx> Timings<'gctx> {
189222
}
190223

191224
/// Mark that a unit has started running.
192-
pub fn unit_start(&mut self, id: JobId, unit: Unit) {
225+
pub fn unit_start(&mut self, build_runner: &BuildRunner<'_, '_>, id: JobId, unit: Unit) {
193226
if !self.enabled {
194227
return;
195228
}
@@ -210,21 +243,36 @@ impl<'gctx> Timings<'gctx> {
210243
CompileMode::Docscrape => target.push_str(" (doc scrape)"),
211244
CompileMode::RunCustomBuild => target.push_str(" (run)"),
212245
}
246+
let start = self.start.elapsed().as_secs_f64();
213247
let unit_time = UnitTime {
214248
unit,
215249
target,
216-
start: self.start.elapsed().as_secs_f64(),
250+
start,
217251
duration: 0.0,
218252
rmeta_time: None,
219-
unlocked_units: Vec::new(),
220-
unlocked_rmeta_units: Vec::new(),
253+
unblocked_units: Vec::new(),
254+
unblocked_rmeta_units: Vec::new(),
221255
sections: Default::default(),
222256
};
257+
if let Some(logger) = build_runner.bcx.logger {
258+
logger.log(LogMessage::UnitStarted {
259+
package_id: unit_time.unit.pkg.package_id().to_spec(),
260+
target: (&unit_time.unit.target).into(),
261+
mode: unit_time.unit.mode,
262+
index: self.unit_to_index[&unit_time.unit],
263+
elapsed: start,
264+
});
265+
}
223266
assert!(self.active.insert(id, unit_time).is_none());
224267
}
225268

226269
/// Mark that the `.rmeta` file as generated.
227-
pub fn unit_rmeta_finished(&mut self, id: JobId, unlocked: Vec<&Unit>) {
270+
pub fn unit_rmeta_finished(
271+
&mut self,
272+
build_runner: &BuildRunner<'_, '_>,
273+
id: JobId,
274+
unblocked: Vec<&Unit>,
275+
) {
228276
if !self.enabled {
229277
return;
230278
}
@@ -234,20 +282,29 @@ impl<'gctx> Timings<'gctx> {
234282
let Some(unit_time) = self.active.get_mut(&id) else {
235283
return;
236284
};
237-
let t = self.start.elapsed().as_secs_f64();
238-
unit_time.rmeta_time = Some(t - unit_time.start);
239-
assert!(unit_time.unlocked_rmeta_units.is_empty());
285+
let elapsed = self.start.elapsed().as_secs_f64();
286+
unit_time.rmeta_time = Some(elapsed - unit_time.start);
287+
assert!(unit_time.unblocked_rmeta_units.is_empty());
240288
unit_time
241-
.unlocked_rmeta_units
242-
.extend(unlocked.iter().cloned().cloned());
289+
.unblocked_rmeta_units
290+
.extend(unblocked.iter().cloned().cloned());
291+
292+
if let Some(logger) = build_runner.bcx.logger {
293+
let unblocked = unblocked.iter().map(|u| self.unit_to_index[u]).collect();
294+
logger.log(LogMessage::UnitRmetaFinished {
295+
index: self.unit_to_index[&unit_time.unit],
296+
elapsed,
297+
unblocked,
298+
});
299+
}
243300
}
244301

245302
/// Mark that a unit has finished running.
246303
pub fn unit_finished(
247304
&mut self,
248305
build_runner: &BuildRunner<'_, '_>,
249306
id: JobId,
250-
unlocked: Vec<&Unit>,
307+
unblocked: Vec<&Unit>,
251308
) {
252309
if !self.enabled {
253310
return;
@@ -256,12 +313,12 @@ impl<'gctx> Timings<'gctx> {
256313
let Some(mut unit_time) = self.active.remove(&id) else {
257314
return;
258315
};
259-
let t = self.start.elapsed().as_secs_f64();
260-
unit_time.duration = t - unit_time.start;
261-
assert!(unit_time.unlocked_units.is_empty());
316+
let elapsed = self.start.elapsed().as_secs_f64();
317+
unit_time.duration = elapsed - unit_time.start;
318+
assert!(unit_time.unblocked_units.is_empty());
262319
unit_time
263-
.unlocked_units
264-
.extend(unlocked.iter().cloned().cloned());
320+
.unblocked_units
321+
.extend(unblocked.iter().cloned().cloned());
265322
if self.report_json {
266323
let msg = machine_message::TimingInfo {
267324
package_id: unit_time.unit.pkg.package_id().to_spec(),
@@ -275,36 +332,56 @@ impl<'gctx> Timings<'gctx> {
275332
crate::drop_println!(self.gctx, "{}", msg);
276333
}
277334
if let Some(logger) = build_runner.bcx.logger {
278-
logger.log(LogMessage::TimingInfo {
279-
package_id: unit_time.unit.pkg.package_id().to_spec(),
280-
target: unit_time.unit.target.clone(),
281-
mode: unit_time.unit.mode,
282-
duration: unit_time.duration,
283-
rmeta_time: unit_time.rmeta_time,
284-
sections: unit_time.sections.clone().into_iter().collect(),
335+
let unblocked = unblocked.iter().map(|u| self.unit_to_index[u]).collect();
336+
logger.log(LogMessage::UnitFinished {
337+
index: self.unit_to_index[&unit_time.unit],
338+
elapsed,
339+
unblocked,
285340
});
286341
}
287342
self.unit_times.push(unit_time);
288343
}
289344

290345
/// Handle the start/end of a compilation section.
291-
pub fn unit_section_timing(&mut self, id: JobId, section_timing: &SectionTiming) {
346+
pub fn unit_section_timing(
347+
&mut self,
348+
build_runner: &BuildRunner<'_, '_>,
349+
id: JobId,
350+
section_timing: &SectionTiming,
351+
) {
292352
if !self.enabled {
293353
return;
294354
}
295355
let Some(unit_time) = self.active.get_mut(&id) else {
296356
return;
297357
};
298-
let now = self.start.elapsed().as_secs_f64();
358+
let elapsed = self.start.elapsed().as_secs_f64();
299359

300360
match section_timing.event {
301361
SectionTimingEvent::Start => {
302-
unit_time.start_section(&section_timing.name, now);
362+
unit_time.start_section(&section_timing.name, elapsed);
303363
}
304364
SectionTimingEvent::End => {
305-
unit_time.end_section(&section_timing.name, now);
365+
unit_time.end_section(&section_timing.name, elapsed);
306366
}
307367
}
368+
369+
if let Some(logger) = build_runner.bcx.logger {
370+
let index = self.unit_to_index[&unit_time.unit];
371+
let section = section_timing.name.clone();
372+
logger.log(match section_timing.event {
373+
SectionTimingEvent::Start => LogMessage::UnitSectionStarted {
374+
index,
375+
elapsed,
376+
section,
377+
},
378+
SectionTimingEvent::End => LogMessage::UnitSectionFinished {
379+
index,
380+
elapsed,
381+
section,
382+
},
383+
})
384+
}
308385
}
309386

310387
/// This is called periodically to mark the concurrency of internal structures.

src/cargo/core/compiler/timings/report.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -373,13 +373,13 @@ fn to_unit_data(unit_times: &[UnitTime]) -> Vec<UnitData> {
373373
// These filter on the unlocked units because not all unlocked
374374
// units are actually "built". For example, Doctest mode units
375375
// don't actually generate artifacts.
376-
let unlocked_units: Vec<usize> = ut
377-
.unlocked_units
376+
let unblocked_units: Vec<usize> = ut
377+
.unblocked_units
378378
.iter()
379379
.filter_map(|unit| unit_map.get(unit).copied())
380380
.collect();
381-
let unlocked_rmeta_units: Vec<usize> = ut
382-
.unlocked_rmeta_units
381+
let unblocked_rmeta_units: Vec<usize> = ut
382+
.unblocked_rmeta_units
383383
.iter()
384384
.filter_map(|unit| unit_map.get(unit).copied())
385385
.collect();
@@ -419,8 +419,8 @@ fn to_unit_data(unit_times: &[UnitTime]) -> Vec<UnitData> {
419419
start: round(ut.start),
420420
duration: round(ut.duration),
421421
rmeta_time: ut.rmeta_time.map(round),
422-
unlocked_units,
423-
unlocked_rmeta_units,
422+
unblocked_units,
423+
unblocked_rmeta_units,
424424
sections,
425425
}
426426
})

0 commit comments

Comments
 (0)