Skip to content
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

Stabilize -Ztimings as --timings #10245

Merged
merged 7 commits into from
Feb 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/bin/cargo/commands/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/rustdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
12 changes: 12 additions & 0 deletions src/cargo/core/compiler/build_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -86,6 +88,7 @@ impl BuildConfig {
rustfix_diagnostic_server: RefCell::new(None),
export_dir: None,
future_incompat_report: false,
timing_outputs: Vec::new(),
})
}

Expand Down Expand Up @@ -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,
}
4 changes: 2 additions & 2 deletions src/cargo/core/compiler/job_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down
51 changes: 14 additions & 37 deletions src/cargo/core/compiler/timings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand Down Expand Up @@ -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)?;
Expand Down
12 changes: 3 additions & 9 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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)?,
Expand Down Expand Up @@ -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),
}

Expand Down
41 changes: 40 additions & 1 deletion src/cargo/util/command_prelude.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)?;
joshtriplett marked this conversation as resolved.
Show resolved Hide resolved
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()
Expand Down
2 changes: 2 additions & 0 deletions src/doc/man/cargo-bench.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ target.

{{> options-ignore-rust-version }}

{{> options-timings }}

{{/options}}

### Output Options
Expand Down
2 changes: 2 additions & 0 deletions src/doc/man/cargo-build.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ they have `required-features` that are missing.

{{> options-ignore-rust-version }}

{{> options-timings }}

{{/options}}

### Output Options
Expand Down
2 changes: 2 additions & 0 deletions src/doc/man/cargo-check.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ they have `required-features` that are missing.

{{> options-ignore-rust-version }}

{{> options-timings }}

{{/options}}

### Output Options
Expand Down
Loading