Skip to content

Commit 5d7b985

Browse files
authored
Rollup merge of #145261 - Kobzol:bootstrap-tracing, r=jieyouxu
Improve tracing in bootstrap I was annoyed that bootstrap had like 5 separate ways of debugging/tracing/profiling, and it was hard for me to understand how are individual steps executed. This PR tries to unify severla things behind `BOOTSTRAP_TRACING`, and improve tracing/profiling in general: - All generated tracing outputs are now stored in a single directory to make it easier to examine them, plus bootstrap prepares a `latest` symlink to the latest generated tracing output directory for convenience. - All executed spans are now logged automatically (without requiring usage of `#[tracing::instrument]`). - A custom span/event formatter was implemented, to provide domain-specific output (like location of executed commands or spans) and hopefully also to reduce visual clutter. - `tracing_forest` was removed. While it did some useful postprocessing, it didn't expose enough information for making the dynamic step spans work. - You can now explicitly log steps (`STEP=info`) and/or commands (`COMMAND=info`), to have more granular control over what gets logged. - `print-step-timings` also show when a step starts its execution (not just when it ends it), so that when some step fails in CI, we can actually see what step it was (before we would only see the end of the previous step). - The rustc-dev-guide page on debugging/profiling bootstrap was updated. There are still some things that work outside of tracing (`print-step-timings` and `dump-bootstrap-shims`), but I think that for now this improvement is good enough. I removed the `> step`, `< step` verbose output, because I found it unusable, as verbose bootstrap output also enables verbose Cargo output, and then you simply drown in too much data, and because I think that the new tracing system makes it obsolete (although it does require recompilation with the `tracing` feature). If you want to keep it, happy to revert 690c781. And the information about cached steps is now also shown in the Graphviz step dependency graph. We can modify the tracing output however we want, as we now implement it ourselves. Notably, we could also show exit logs for step spans, currently I only show enter spans. Maybe creating indents for each executed nested command is also not needed. Happy to hear feedback! Some further improvements could be to print step durations, if we decide to also log step exit events. We could also try to enable tracing in CI logs, but it might be too verbose. Best reviewed commit-by-commit. r? `@jieyouxu` CC `@Shourya742`
2 parents b2b332c + 0b8c6ad commit 5d7b985

File tree

13 files changed

+560
-412
lines changed

13 files changed

+560
-412
lines changed

src/bootstrap/Cargo.lock

Lines changed: 25 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,18 @@ dependencies = [
1111
"memchr",
1212
]
1313

14-
[[package]]
15-
name = "ansi_term"
16-
version = "0.12.1"
17-
source = "registry+https://github.com/rust-lang/crates.io-index"
18-
checksum = "d52a9bb7ec0cf484c551830a7ce27bd20d67eac647e1befb56b0be4ee39a55d2"
19-
dependencies = [
20-
"winapi",
21-
]
22-
2314
[[package]]
2415
name = "anstyle"
2516
version = "1.0.10"
2617
source = "registry+https://github.com/rust-lang/crates.io-index"
2718
checksum = "55cc3b69f167a1ef2e161439aa98aed94e6028e5f9a59be9a6ffb47aef1651f9"
2819

20+
[[package]]
21+
name = "autocfg"
22+
version = "1.5.0"
23+
source = "registry+https://github.com/rust-lang/crates.io-index"
24+
checksum = "c08606f8c3cbf4ce6ec8e28fb0014a2c086708fe954eaa885384a6165172e7e8"
25+
2926
[[package]]
3027
name = "bitflags"
3128
version = "2.6.0"
@@ -47,6 +44,7 @@ version = "0.0.0"
4744
dependencies = [
4845
"build_helper",
4946
"cc",
47+
"chrono",
5048
"clap",
5149
"clap_complete",
5250
"cmake",
@@ -71,7 +69,6 @@ dependencies = [
7169
"toml",
7270
"tracing",
7371
"tracing-chrome",
74-
"tracing-forest",
7572
"tracing-subscriber",
7673
"walkdir",
7774
"windows",
@@ -112,6 +109,15 @@ version = "1.0.0"
112109
source = "registry+https://github.com/rust-lang/crates.io-index"
113110
checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd"
114111

112+
[[package]]
113+
name = "chrono"
114+
version = "0.4.41"
115+
source = "registry+https://github.com/rust-lang/crates.io-index"
116+
checksum = "c469d952047f47f91b68d1cba3f10d63c11d73e4636f24f08daf0278abf01c4d"
117+
dependencies = [
118+
"num-traits",
119+
]
120+
115121
[[package]]
116122
name = "clap"
117123
version = "4.5.20"
@@ -458,6 +464,15 @@ dependencies = [
458464
"winapi",
459465
]
460466

467+
[[package]]
468+
name = "num-traits"
469+
version = "0.2.19"
470+
source = "registry+https://github.com/rust-lang/crates.io-index"
471+
checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841"
472+
dependencies = [
473+
"autocfg",
474+
]
475+
461476
[[package]]
462477
name = "objc2-core-foundation"
463478
version = "0.3.1"
@@ -775,26 +790,6 @@ dependencies = [
775790
"winapi-util",
776791
]
777792

778-
[[package]]
779-
name = "thiserror"
780-
version = "1.0.69"
781-
source = "registry+https://github.com/rust-lang/crates.io-index"
782-
checksum = "b6aaf5339b578ea85b50e080feb250a3e8ae8cfcdff9a461c9ec2904bc923f52"
783-
dependencies = [
784-
"thiserror-impl",
785-
]
786-
787-
[[package]]
788-
name = "thiserror-impl"
789-
version = "1.0.69"
790-
source = "registry+https://github.com/rust-lang/crates.io-index"
791-
checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1"
792-
dependencies = [
793-
"proc-macro2",
794-
"quote",
795-
"syn",
796-
]
797-
798793
[[package]]
799794
name = "thread_local"
800795
version = "1.1.8"
@@ -857,19 +852,6 @@ dependencies = [
857852
"valuable",
858853
]
859854

860-
[[package]]
861-
name = "tracing-forest"
862-
version = "0.1.6"
863-
source = "registry+https://github.com/rust-lang/crates.io-index"
864-
checksum = "ee40835db14ddd1e3ba414292272eddde9dad04d3d4b65509656414d1c42592f"
865-
dependencies = [
866-
"ansi_term",
867-
"smallvec",
868-
"thiserror",
869-
"tracing",
870-
"tracing-subscriber",
871-
]
872-
873855
[[package]]
874856
name = "tracing-log"
875857
version = "0.2.0"

src/bootstrap/Cargo.toml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ default-run = "bootstrap"
77

88
[features]
99
build-metrics = ["sysinfo"]
10-
tracing = ["dep:tracing", "dep:tracing-chrome", "dep:tracing-subscriber", "dep:tracing-forest"]
10+
tracing = ["dep:tracing", "dep:tracing-chrome", "dep:tracing-subscriber", "dep:chrono", "dep:tempfile"]
1111

1212
[lib]
1313
path = "src/lib.rs"
@@ -61,10 +61,11 @@ xz2 = "0.1"
6161
sysinfo = { version = "0.37.0", default-features = false, optional = true, features = ["system"] }
6262

6363
# Dependencies needed by the `tracing` feature
64+
chrono = { version = "0.4", default-features = false, optional = true, features = ["now", "std"] }
6465
tracing = { version = "0.1", optional = true, features = ["attributes"] }
6566
tracing-chrome = { version = "0.7", optional = true }
6667
tracing-subscriber = { version = "0.3", optional = true, features = ["env-filter", "fmt", "registry", "std"] }
67-
tracing-forest = { version = "0.1.6", optional = true, default-features = false, features = ["smallvec", "ansi", "env-filter"] }
68+
tempfile = { version = "3.15.0", optional = true }
6869

6970
[target.'cfg(windows)'.dependencies.junction]
7071
version = "1.0.0"

src/bootstrap/src/bin/main.rs

Lines changed: 40 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
88
use std::fs::{self, OpenOptions};
99
use std::io::{self, BufRead, BufReader, IsTerminal, Write};
10+
use std::path::Path;
1011
use std::str::FromStr;
1112
use std::time::Instant;
1213
use std::{env, process};
@@ -15,19 +16,16 @@ use bootstrap::{
1516
Build, CONFIG_CHANGE_HISTORY, ChangeId, Config, Flags, Subcommand, debug,
1617
find_recent_config_change_ids, human_readable_changes, t,
1718
};
18-
#[cfg(feature = "tracing")]
19-
use tracing::instrument;
2019

21-
fn is_bootstrap_profiling_enabled() -> bool {
22-
env::var("BOOTSTRAP_PROFILE").is_ok_and(|v| v == "1")
20+
fn is_tracing_enabled() -> bool {
21+
cfg!(feature = "tracing")
2322
}
2423

25-
#[cfg_attr(feature = "tracing", instrument(level = "trace", name = "main"))]
2624
fn main() {
2725
#[cfg(feature = "tracing")]
28-
let _guard = setup_tracing();
26+
let guard = bootstrap::setup_tracing("BOOTSTRAP_TRACING");
2927

30-
let start_time = Instant::now();
28+
let _start_time = Instant::now();
3129

3230
let args = env::args().skip(1).collect::<Vec<_>>();
3331

@@ -102,6 +100,35 @@ fn main() {
102100
let dump_bootstrap_shims = config.dump_bootstrap_shims;
103101
let out_dir = config.out.clone();
104102

103+
let tracing_enabled = is_tracing_enabled();
104+
105+
// Prepare a directory for tracing output
106+
// Also store a symlink named "latest" to point to the latest tracing directory.
107+
let tracing_dir = out_dir.join("bootstrap-trace").join(std::process::id().to_string());
108+
let latest_trace_dir = tracing_dir.parent().unwrap().join("latest");
109+
if tracing_enabled {
110+
let _ = std::fs::remove_dir_all(&tracing_dir);
111+
std::fs::create_dir_all(&tracing_dir).unwrap();
112+
113+
#[cfg(windows)]
114+
let _ = std::fs::remove_dir(&latest_trace_dir);
115+
#[cfg(not(windows))]
116+
let _ = std::fs::remove_file(&latest_trace_dir);
117+
118+
#[cfg(not(windows))]
119+
fn symlink_dir_inner(original: &Path, link: &Path) -> io::Result<()> {
120+
use std::os::unix::fs;
121+
fs::symlink(original, link)
122+
}
123+
124+
#[cfg(windows)]
125+
fn symlink_dir_inner(target: &Path, junction: &Path) -> io::Result<()> {
126+
junction::create(target, junction)
127+
}
128+
129+
t!(symlink_dir_inner(&tracing_dir, &latest_trace_dir));
130+
}
131+
105132
debug!("creating new build based on config");
106133
let mut build = Build::new(config);
107134
build.build();
@@ -156,12 +183,13 @@ fn main() {
156183
}
157184
}
158185

159-
if is_bootstrap_profiling_enabled() {
160-
build.report_summary(start_time);
161-
}
162-
163186
#[cfg(feature = "tracing")]
164-
build.report_step_graph();
187+
{
188+
build.report_summary(&tracing_dir.join("command-stats.txt"), _start_time);
189+
build.report_step_graph(&tracing_dir);
190+
guard.copy_to_dir(&tracing_dir);
191+
eprintln!("Tracing/profiling output has been written to {}", latest_trace_dir.display());
192+
}
165193
}
166194

167195
fn check_version(config: &Config) -> Option<String> {
@@ -219,37 +247,3 @@ fn check_version(config: &Config) -> Option<String> {
219247

220248
Some(msg)
221249
}
222-
223-
// # Note on `tracing` usage in bootstrap
224-
//
225-
// Due to the conditional compilation via the `tracing` cargo feature, this means that `tracing`
226-
// usages in bootstrap need to be also gated behind the `tracing` feature:
227-
//
228-
// - `tracing` macros with log levels (`trace!`, `debug!`, `warn!`, `info`, `error`) should not be
229-
// used *directly*. You should use the wrapped `tracing` macros which gate the actual invocations
230-
// behind `feature = "tracing"`.
231-
// - `tracing`'s `#[instrument(..)]` macro will need to be gated like `#![cfg_attr(feature =
232-
// "tracing", instrument(..))]`.
233-
#[cfg(feature = "tracing")]
234-
fn setup_tracing() -> impl Drop {
235-
use tracing_forest::ForestLayer;
236-
use tracing_subscriber::EnvFilter;
237-
use tracing_subscriber::layer::SubscriberExt;
238-
239-
let filter = EnvFilter::from_env("BOOTSTRAP_TRACING");
240-
241-
let mut chrome_layer = tracing_chrome::ChromeLayerBuilder::new().include_args(true);
242-
243-
// Writes the Chrome profile to trace-<unix-timestamp>.json if enabled
244-
if !is_bootstrap_profiling_enabled() {
245-
chrome_layer = chrome_layer.writer(io::sink());
246-
}
247-
248-
let (chrome_layer, _guard) = chrome_layer.build();
249-
250-
let registry =
251-
tracing_subscriber::registry().with(filter).with(ForestLayer::default()).with(chrome_layer);
252-
253-
tracing::subscriber::set_global_default(registry).unwrap();
254-
_guard
255-
}

0 commit comments

Comments
 (0)