Auto merge of #10245 - joshtriplett:stabilize-timings, r=joshtriplett

Stabilize `-Ztimings` as `--timings`

The `-Ztimings` option has existed for years, and many people use it to
profile and optimize their builds. It's one of the common reasons people
use nightly cargo.

The machine-readable JSON output may warrant further careful inspection
before we commit to a stable format. However, for the human-readable
output we don't need to make any commitment about the exact output.

Add a `--timings` option, as the stable equivalent to `-Ztimings`.
(Passing `html` or `json` requires `-Zunstable-options`, but the default `--timings` does not.)

Document the new option, and update the testsuite.

src/bin/cargo/commands/bench.rs

@@ -47,6 +47,7 @@ pub fn cli() -> App {
             "Run all benchmarks regardless of failure",
         ))
         .arg_unit_graph()
+        .arg_timings()
         .after_help("Run `cargo help bench` for more detailed information.\n")
 }
 

src/bin/cargo/commands/build.rs

@@ -44,6 +44,7 @@ pub fn cli() -> App {
         .arg_build_plan()
         .arg_unit_graph()
         .arg_future_incompat_report()
+        .arg_timings()
         .after_help("Run `cargo help build` for more detailed information.\n")
 }
 

src/bin/cargo/commands/check.rs

@@ -36,6 +36,7 @@ pub fn cli() -> App {
         .arg_message_format()
         .arg_unit_graph()
         .arg_future_incompat_report()
+        .arg_timings()
         .after_help("Run `cargo help check` for more detailed information.\n")
 }
 

src/bin/cargo/commands/doc.rs

@@ -36,6 +36,7 @@ pub fn cli() -> App {
         .arg_message_format()
         .arg_ignore_rust_version()
         .arg_unit_graph()
+        .arg_timings()
         .after_help("Run `cargo help doc` for more detailed information.\n")
 }
 

src/bin/cargo/commands/fix.rs

@@ -62,6 +62,7 @@ pub fn cli() -> App {
                 .help("Fix code even if the working directory has staged changes"),
         )
         .arg_ignore_rust_version()
+        .arg_timings()
         .after_help("Run `cargo help fix` for more detailed information.\n")
 }
 

src/bin/cargo/commands/install.rs

@@ -78,6 +78,7 @@ pub fn cli() -> App {
                 .conflicts_with_all(&["git", "path", "index"]),
         )
         .arg_message_format()
+        .arg_timings()
         .after_help("Run `cargo help install` for more detailed information.\n")
 }
 

src/bin/cargo/commands/run.rs

@@ -31,6 +31,7 @@ pub fn cli() -> App {
         .arg_message_format()
         .arg_unit_graph()
         .arg_ignore_rust_version()
+        .arg_timings()
         .after_help("Run `cargo help run` for more detailed information.\n")
 }
 

src/bin/cargo/commands/rustc.rs

@@ -47,6 +47,7 @@ pub fn cli() -> App {
         .arg_unit_graph()
         .arg_ignore_rust_version()
         .arg_future_incompat_report()
+        .arg_timings()
         .after_help("Run `cargo help rustc` for more detailed information.\n")
 }
 

src/bin/cargo/commands/rustdoc.rs

@@ -35,6 +35,7 @@ pub fn cli() -> App {
         .arg_message_format()
         .arg_unit_graph()
         .arg_ignore_rust_version()
+        .arg_timings()
         .after_help("Run `cargo help rustdoc` for more detailed information.\n")
 }
 

src/bin/cargo/commands/test.rs

@@ -56,6 +56,7 @@ pub fn cli() -> App {
         .arg_message_format()
         .arg_unit_graph()
         .arg_future_incompat_report()
+        .arg_timings()
         .after_help(
             "Run `cargo help test` for more detailed information.\n\
              Run `cargo test -- --help` for test binary options.\n",

src/cargo/core/compiler/build_config.rs

@@ -39,6 +39,8 @@ pub struct BuildConfig {
     pub export_dir: Option<PathBuf>,
     /// `true` to output a future incompatibility report at the end of the build
     pub future_incompat_report: bool,
+    /// Which kinds of build timings to output (empty if none).
+    pub timing_outputs: Vec<TimingOutput>,
 }
 
 impl BuildConfig {
@@ -86,6 +88,7 @@ impl BuildConfig {
             rustfix_diagnostic_server: RefCell::new(None),
             export_dir: None,
             future_incompat_report: false,
+            timing_outputs: Vec::new(),
         })
     }
 
@@ -231,3 +234,12 @@ impl CompileMode {
         )
     }
 }
+
+/// Kinds of build timings we can output.
+#[derive(Clone, Copy, PartialEq, Debug, Eq, Hash, PartialOrd, Ord)]
+pub enum TimingOutput {
+    /// Human-readable HTML report
+    Html,
+    /// Machine-readable JSON (unstable)
+    Json,
+}

src/cargo/core/compiler/job_queue.rs

@@ -839,7 +839,7 @@ impl<'cfg> DrainState<'cfg> {
         }
 
         let time_elapsed = util::elapsed(cx.bcx.config.creation_time().elapsed());
-        if let Err(e) = self.timings.finished(cx.bcx, &error) {
+        if let Err(e) = self.timings.finished(cx, &error) {
             if error.is_some() {
                 crate::display_error(&e, &mut cx.bcx.config.shell());
             } else {
@@ -906,7 +906,7 @@ impl<'cfg> DrainState<'cfg> {
     // this as often as we spin on the events receiver (at least every 500ms or
     // so).
     fn tick_progress(&mut self) {
-        // Record some timing information if `-Ztimings` is enabled, and
+        // Record some timing information if `--timings` is enabled, and
         // this'll end up being a noop if we're not recording this
         // information.
         self.timings.mark_concurrency(

src/cargo/core/compiler/mod.rs

@@ -33,7 +33,7 @@ use anyhow::{Context as _, Error};
 use lazycell::LazyCell;
 use log::{debug, trace};
 
-pub use self::build_config::{BuildConfig, CompileMode, MessageFormat};
+pub use self::build_config::{BuildConfig, CompileMode, MessageFormat, TimingOutput};
 pub use self::build_context::{
     BuildContext, FileFlavor, FileType, RustDocFingerprint, RustcTargetData, TargetInfo,
 };

src/cargo/core/compiler/timings.rs

@@ -4,7 +4,7 @@
 //! long it takes for different units to compile.
 use super::{CompileMode, Unit};
 use crate::core::compiler::job_queue::JobId;
-use crate::core::compiler::BuildContext;
+use crate::core::compiler::{BuildContext, Context, TimingOutput};
 use crate::core::PackageId;
 use crate::util::cpu::State;
 use crate::util::machine_message::{self, Message};
@@ -21,8 +21,6 @@ pub struct Timings<'cfg> {
     enabled: bool,
     /// If true, saves an HTML report to disk.
     report_html: bool,
-    /// If true, reports unit completion to stderr.
-    report_info: bool,
     /// If true, emits JSON information with timing information.
     report_json: bool,
     /// When Cargo started.
@@ -94,17 +92,10 @@ struct Concurrency {
 
 impl<'cfg> Timings<'cfg> {
     pub fn new(bcx: &BuildContext<'_, 'cfg>, root_units: &[Unit]) -> Timings<'cfg> {
-        let has_report = |what| {
-            bcx.config
-                .cli_unstable()
-                .timings
-                .as_ref()
-                .map_or(false, |t| t.iter().any(|opt| opt == what))
-        };
-        let report_html = has_report("html");
-        let report_info = has_report("info");
-        let report_json = has_report("json");
-        let enabled = report_html | report_info | report_json;
+        let has_report = |what| bcx.build_config.timing_outputs.contains(&what);
+        let report_html = has_report(TimingOutput::Html);
+        let report_json = has_report(TimingOutput::Json);
+        let enabled = report_html | report_json;
 
         let mut root_map: HashMap<PackageId, Vec<String>> = HashMap::new();
         for unit in root_units {
@@ -139,7 +130,6 @@ impl<'cfg> Timings<'cfg> {
             config: bcx.config,
             enabled,
             report_html,
-            report_info,
             report_json,
             start: bcx.config.creation_time(),
             start_str,
@@ -227,18 +217,6 @@ impl<'cfg> Timings<'cfg> {
         unit_time
             .unlocked_units
             .extend(unlocked.iter().cloned().cloned());
-        if self.report_info {
-            let msg = format!(
-                "{}{} in {:.1}s",
-                unit_time.name_ver(),
-                unit_time.target,
-                unit_time.duration
-            );
-            let _ = self
-                .config
-                .shell()
-                .status_with_color("Completed", msg, termcolor::Color::Cyan);
-        }
         if self.report_json {
             let msg = machine_message::TimingInfo {
                 package_id: unit_time.unit.pkg.package_id(),
@@ -315,7 +293,7 @@ impl<'cfg> Timings<'cfg> {
     /// Call this when all units are finished.
     pub fn finished(
         &mut self,
-        bcx: &BuildContext<'_, '_>,
+        cx: &Context<'_, '_>,
         error: &Option<anyhow::Error>,
     ) -> CargoResult<()> {
         if !self.enabled {
@@ -325,29 +303,27 @@ impl<'cfg> Timings<'cfg> {
         self.unit_times
             .sort_unstable_by(|a, b| a.start.partial_cmp(&b.start).unwrap());
         if self.report_html {
-            self.report_html(bcx, error)
+            self.report_html(cx, error)
                 .with_context(|| "failed to save timing report")?;
         }
         Ok(())
     }
 
     /// Save HTML report to disk.
-    fn report_html(
-        &self,
-        bcx: &BuildContext<'_, '_>,
-        error: &Option<anyhow::Error>,
-    ) -> CargoResult<()> {
+    fn report_html(&self, cx: &Context<'_, '_>, error: &Option<anyhow::Error>) -> CargoResult<()> {
         let duration = self.start.elapsed().as_secs_f64();
         let timestamp = self.start_str.replace(&['-', ':'][..], "");
-        let filename = format!("cargo-timing-{}.html", timestamp);
+        let timings_path = cx.files().host_root().join("cargo-timings");
+        paths::create_dir_all(&timings_path)?;
+        let filename = timings_path.join(format!("cargo-timing-{}.html", timestamp));
         let mut f = BufWriter::new(paths::create(&filename)?);
         let roots: Vec<&str> = self
             .root_targets
             .iter()
             .map(|(name, _targets)| name.as_str())
             .collect();
         f.write_all(HTML_TMPL.replace("{ROOTS}", &roots.join(", ")).as_bytes())?;
-        self.write_summary_table(&mut f, duration, bcx, error)?;
+        self.write_summary_table(&mut f, duration, cx.bcx, error)?;
         f.write_all(HTML_CANVAS.as_bytes())?;
         self.write_unit_table(&mut f)?;
         // It helps with pixel alignment to use whole numbers.
@@ -375,7 +351,8 @@ impl<'cfg> Timings<'cfg> {
                 .join(&filename)
                 .display()
         );
-        paths::link_or_copy(&filename, "cargo-timing.html")?;
+        let unstamped_filename = timings_path.join("cargo-timing.html");
+        paths::link_or_copy(&filename, &unstamped_filename)?;
         self.config
             .shell()
             .status_with_color("Timing", msg, termcolor::Color::Cyan)?;

src/cargo/core/features.rs

@@ -652,7 +652,6 @@ unstable_cli_options!(
     rustdoc_map: bool = ("Allow passing external documentation mappings to rustdoc"),
     separate_nightlies: bool = (HIDDEN),
     terminal_width: Option<Option<usize>>  = ("Provide a terminal width to rustc for error truncation"),
-    timings: Option<Vec<String>>  = ("Display concurrency information"),
     unstable_options: bool = ("Allow the usage of unstable options"),
     // TODO(wcrichto): move scrape example configuration into Cargo.toml before stabilization
     // See: https://github.com/rust-lang/cargo/pull/9525#discussion_r728470927
@@ -712,6 +711,8 @@ const STABILIZED_WEAK_DEP_FEATURES: &str = "Weak dependency features are now alw
 
 const STABILISED_NAMESPACED_FEATURES: &str = "Namespaced features are now always available.";
 
+const STABILIZED_TIMINGS: &str = "The -Ztimings option has been stabilized as --timings.";
+
 fn deserialize_build_std<'de, D>(deserializer: D) -> Result<Option<Vec<String>>, D::Error>
 where
     D: serde::Deserializer<'de>,
@@ -768,13 +769,6 @@ impl CliUnstable {
             }
         }
 
-        fn parse_timings(value: Option<&str>) -> Vec<String> {
-            match value {
-                None => vec!["html".to_string(), "info".to_string()],
-                Some(v) => v.split(',').map(|s| s.to_string()).collect(),
-            }
-        }
-
         fn parse_features(value: Option<&str>) -> Vec<String> {
             match value {
                 None => Vec::new(),
@@ -849,7 +843,6 @@ impl CliUnstable {
                 self.build_std = Some(crate::core::compiler::standard_lib::parse_unstable_flag(v))
             }
             "build-std-features" => self.build_std_features = Some(parse_features(v)),
-            "timings" => self.timings = Some(parse_timings(v)),
             "doctest-xcompile" => self.doctest_xcompile = parse_empty(k, v)?,
             "doctest-in-workspace" => self.doctest_in_workspace = parse_empty(k, v)?,
             "panic-abort-tests" => self.panic_abort_tests = parse_empty(k, v)?,
@@ -904,6 +897,7 @@ impl CliUnstable {
             "future-incompat-report" => {
                 stabilized_warn(k, "1.59.0", STABILIZED_FUTURE_INCOMPAT_REPORT)
             }
+            "timings" => stabilized_warn(k, "1.60", STABILIZED_TIMINGS),
             _ => bail!("unknown `-Z` flag specified: {}", k),
         }
 

src/cargo/util/command_prelude.rs

@@ -1,4 +1,4 @@
-use crate::core::compiler::{BuildConfig, MessageFormat};
+use crate::core::compiler::{BuildConfig, MessageFormat, TimingOutput};
 use crate::core::resolver::CliFeatures;
 use crate::core::{Edition, Workspace};
 use crate::ops::{CompileFilter, CompileOptions, NewOptions, Packages, VersionControl};
@@ -234,6 +234,17 @@ pub trait AppExt: Sized {
     fn arg_quiet(self) -> Self {
         self._arg(opt("quiet", "Do not print cargo log messages").short('q'))
     }
+
+    fn arg_timings(self) -> Self {
+        self._arg(
+            optional_opt(
+                "timings",
+                "Timing output formats (unstable) (comma separated): html, json",
+            )
+            .value_name("FMTS")
+            .require_equals(true),
+        )
+    }
 }
 
 impl AppExt for App {
@@ -499,6 +510,34 @@ pub trait ArgMatchesExt {
         build_config.build_plan = self.is_valid_and_present("build-plan");
         build_config.unit_graph = self.is_valid_and_present("unit-graph");
         build_config.future_incompat_report = self.is_valid_and_present("future-incompat-report");
+
+        if self.is_valid_and_present("timings") {
+            for timing_output in self._values_of("timings") {
+                for timing_output in timing_output.split(',') {
+                    let timing_output = timing_output.to_ascii_lowercase();
+                    let timing_output = match timing_output.as_str() {
+                        "html" => {
+                            config
+                                .cli_unstable()
+                                .fail_if_stable_opt("--timings=html", 7405)?;
+                            TimingOutput::Html
+                        }
+                        "json" => {
+                            config
+                                .cli_unstable()
+                                .fail_if_stable_opt("--timings=json", 7405)?;
+                            TimingOutput::Json
+                        }
+                        s => bail!("invalid timings output specifier: `{}`", s),
+                    };
+                    build_config.timing_outputs.push(timing_output);
+                }
+            }
+            if build_config.timing_outputs.is_empty() {
+                build_config.timing_outputs.push(TimingOutput::Html);
+            }
+        }
+
         if build_config.build_plan {
             config
                 .cli_unstable()

src/doc/man/cargo-bench.md

@@ -95,6 +95,8 @@ target.
 
 {{> options-ignore-rust-version }}
 
+{{> options-timings }}
+
 {{/options}}
 
 ### Output Options

src/doc/man/cargo-build.md

@@ -39,6 +39,8 @@ they have `required-features` that are missing.
 
 {{> options-ignore-rust-version }}
 
+{{> options-timings }}
+
 {{/options}}
 
 ### Output Options

src/doc/man/cargo-check.md

@@ -44,6 +44,8 @@ they have `required-features` that are missing.
 
 {{> options-ignore-rust-version }}
 
+{{> options-timings }}
+
 {{/options}}
 
 ### Output Options