feat(opt-dist): new flag `--benchmark-cargo-config` #125930

weihanglo · 2024-06-03T16:00:18Z

This should be the last piece toward self-contained opt-dist (I believe).

The flag propagates cargo configs to rustc-perf --cargo-config,
which is particularly useful when the environment is air-gapped,
and you want to use the default set of training crates vendored
in the rustc-src tarball.

It fixes the issue described in #125465

The current pinned rustc-perf uses tempfile::Tempdir as the working
directory when collecting profiles from some of these packages.
This "tmp" working directory usage make it impossible for Cargo to pick
up the correct vendor sources setting in .cargo/config.toml bundled
in the rustc-src tarball. ¹

See also

r? Kobzol

https://github.com/rust-lang/rustc-perf/blob/4f313add609f43e928e98132358e8426ed3969ae/collector/src/compile/benchmark/mod.rs#L164-L173 ↩

This is needed for fixing the missing license issue. See rust-lang#125465.

rustbot · 2024-06-03T16:00:27Z

r? @Kobzol

rustbot has assigned @Kobzol.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-06-03T16:00:27Z

⚠️ Warning ⚠️

These commits modify submodules.

weihanglo · 2024-06-03T16:03:53Z

blocked on #125465 btw

weihanglo · 2024-06-03T16:45:30Z

@bors try @rust-timer queue

…fig, r=<try> feat(opt-dist): respect existing .cargo/config.toml This should be the last piece toward self-contained `opt-dist` (I believe). It fixes the issue described in rust-lang#125465 > * The current pinned rustc-perf uses `tempfile::Tempdir` as the working directory when collecting profiles from some of these packages. This "tmp" working directory usage make it impossible for Cargo to pick up the correct vendor sources setting in `.cargo/config.toml` bundled in the rustc-src tarball. [^1] > [^1]: https://github.com/rust-lang/rustc-perf/blob/4f313add609f43e928e98132358e8426ed3969ae/collector/src/compile/benchmark/mod.rs#L164-L173 See also * <rust-lang/rustc-perf#1913> * <rust-lang#125465> * https://rust-lang.zulipchat.com/#narrow/stream/247081-t-compiler.2Fperformance/topic/tempfile.20in.20rustc-perf.20make.20it.20hard.20to.20configure.20vendor r? Kobzol

bors · 2024-06-03T16:46:40Z

⌛ Trying commit 89fb909 with merge 8c225b2...

bors · 2024-06-03T16:53:13Z

💔 Test failed - checks-actions

weihanglo · 2024-06-03T16:57:19Z

@bors try @rust-timer queue

bors · 2024-06-03T16:58:29Z

⌛ Trying commit ff6aceb with merge ec4e11a...

…fig, r=<try> feat(opt-dist): respect existing .cargo/config.toml This should be the last piece toward self-contained `opt-dist` (I believe). It fixes the issue described in rust-lang#125465 > * The current pinned rustc-perf uses `tempfile::Tempdir` as the working directory when collecting profiles from some of these packages. This "tmp" working directory usage make it impossible for Cargo to pick up the correct vendor sources setting in `.cargo/config.toml` bundled in the rustc-src tarball. [^1] > [^1]: https://github.com/rust-lang/rustc-perf/blob/4f313add609f43e928e98132358e8426ed3969ae/collector/src/compile/benchmark/mod.rs#L164-L173 See also * <rust-lang/rustc-perf#1913> * <rust-lang#125465> * https://rust-lang.zulipchat.com/#narrow/stream/247081-t-compiler.2Fperformance/topic/tempfile.20in.20rustc-perf.20make.20it.20hard.20to.20configure.20vendor r? Kobzol

bors · 2024-06-03T18:36:44Z

☀️ Try build successful - checks-actions
Build commit: ec4e11a (ec4e11ac138094f281c3404e2cdc3eec0bc273e1)

Kobzol · 2024-06-03T20:28:05Z

I'm not sure if this should be the default, people can have their own .cargo/config.toml files with whatever contents, and ideally dist (and opt-dist) should probably ignore these by default, and be self-contained and isolated. We could gate it behind a CLI flag, and people using vendoring could just use the flag explicitly. What do you think?

weihanglo · 2024-06-03T20:43:10Z

I'm not sure if this should be the default, people can have their own .cargo/config.toml files with whatever contents, and ideally dist (and opt-dist) should probably ignore these by default, and be self-contained and isolated. We could gate it behind a CLI flag, and people using vendoring could just use the flag explicitly. What do you think?

Yeah I've considered a new flag, though not sure how it should look like. Just propagate --cargo-config flag to opt-dist?

Kobzol · 2024-06-03T20:45:30Z

Yeah, that sounds reasonable. We should also document it here (maybe we should add some more recent changes there).

The flag propagates cargo configs to `rustc-perf --cargo-config`, which is particularly useful when the environment is air-gapped, and you want to use the default set of training crates vendored in the rustc-src tarball.

rust-timer · 2024-06-03T21:19:46Z

Finished benchmarking commit (ec4e11a): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (secondary -3.4%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.4%	[-3.4%, -3.4%]	1
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 667.912s -> 667.423s (-0.07%)
Artifact size: 318.98 MiB -> 318.92 MiB (-0.02%)

weihanglo · 2024-06-03T21:25:11Z

@Kobzol. A new flag --benchmark-cargo-config is added(naming is hard 😅). Let me know if it needs more changes :)

Just note that this PR is not really blocked on #125465. However #125465 might need to be merged before a new beta comes out, as we've already started vendoring rustc-perf in some previous PRs.

I'll prepare a doc update soon once everything's settled.

Kobzol

Looks good. Feel free to r=me once you remove the draft status.

rustbot · 2024-06-03T21:28:05Z

Some changes occurred in src/tools/opt-dist

cc @Kobzol

weihanglo · 2024-06-03T21:35:32Z

Should be safe to rollup.

@bors r=Kobzol rollup

bors · 2024-06-03T21:35:34Z

📌 Commit 0a11dcf has been approved by Kobzol

It is now in the queue for this repository.

…onfig, r=Kobzol feat(opt-dist): new flag `--benchmark-cargo-config` This should be the last piece toward self-contained `opt-dist` (I believe). The flag propagates cargo configs to `rustc-perf --cargo-config`, which is particularly useful when the environment is air-gapped, and you want to use the default set of training crates vendored in the rustc-src tarball. It fixes the issue described in rust-lang#125465 > * The current pinned rustc-perf uses `tempfile::Tempdir` as the working directory when collecting profiles from some of these packages. This "tmp" working directory usage make it impossible for Cargo to pick up the correct vendor sources setting in `.cargo/config.toml` bundled in the rustc-src tarball. [^1] > [^1]: https://github.com/rust-lang/rustc-perf/blob/4f313add609f43e928e98132358e8426ed3969ae/collector/src/compile/benchmark/mod.rs#L164-L173 See also * <rust-lang/rustc-perf#1913> * <rust-lang#125465> * https://rust-lang.zulipchat.com/#narrow/stream/247081-t-compiler.2Fperformance/topic/tempfile.20in.20rustc-perf.20make.20it.20hard.20to.20configure.20vendor r? Kobzol

…llaumeGomez Rollup of 11 pull requests Successful merges: - rust-lang#106186 (Add function `core::iter::chain`) - rust-lang#125596 (Convert `proc_macro_back_compat` lint to an unconditional error.) - rust-lang#125696 (Explain differences between `{Once,Lazy}{Cell,Lock}` types) - rust-lang#125917 (Create `run-make` `env_var` and `env_var_os` helpers) - rust-lang#125927 (Ignore `vec_deque_alloc_error::test_shrink_to_unwind` test on non-unwind targets) - rust-lang#125930 (feat(opt-dist): new flag `--benchmark-cargo-config`) - rust-lang#125932 (Fix typo in the docs of `HashMap::raw_entry_mut`) - rust-lang#125933 (Update books) - rust-lang#125944 (Update fuchsia maintainers) - rust-lang#125946 (Include trailing commas in wrapped function declarations [RustDoc]) - rust-lang#125973 (Remove `tests/run-make-fulldeps/pretty-expanded`) Failed merges: - rust-lang#125815 (`rustc_parse` top-level cleanups) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#125930 - weihanglo:opt-dist-respect-cargo-config, r=Kobzol feat(opt-dist): new flag `--benchmark-cargo-config` This should be the last piece toward self-contained `opt-dist` (I believe). The flag propagates cargo configs to `rustc-perf --cargo-config`, which is particularly useful when the environment is air-gapped, and you want to use the default set of training crates vendored in the rustc-src tarball. It fixes the issue described in rust-lang#125465 > * The current pinned rustc-perf uses `tempfile::Tempdir` as the working directory when collecting profiles from some of these packages. This "tmp" working directory usage make it impossible for Cargo to pick up the correct vendor sources setting in `.cargo/config.toml` bundled in the rustc-src tarball. [^1] > [^1]: https://github.com/rust-lang/rustc-perf/blob/4f313add609f43e928e98132358e8426ed3969ae/collector/src/compile/benchmark/mod.rs#L164-L173 See also * <rust-lang/rustc-perf#1913> * <rust-lang#125465> * https://rust-lang.zulipchat.com/#narrow/stream/247081-t-compiler.2Fperformance/topic/tempfile.20in.20rustc-perf.20make.20it.20hard.20to.20configure.20vendor r? Kobzol

…-cargo-config, r=Kobzol" This reverts commit 610592b, reversing changes made to d9e149d.

chore: update src/tools/rustc-perf

e9c4eb3

This is needed for fixing the missing license issue. See rust-lang#125465.

rustbot assigned Kobzol Jun 3, 2024

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 3, 2024

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 3, 2024

This comment has been minimized.

Sign in to view

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 3, 2024

weihanglo force-pushed the opt-dist-respect-cargo-config branch from 89fb909 to ff6aceb Compare June 3, 2024 16:57

This comment has been minimized.

Sign in to view

feat(opt-dist): new flag --benchmark-cargo-config

0a11dcf

The flag propagates cargo configs to `rustc-perf --cargo-config`, which is particularly useful when the environment is air-gapped, and you want to use the default set of training crates vendored in the rustc-src tarball.

weihanglo force-pushed the opt-dist-respect-cargo-config branch from ff6aceb to 0a11dcf Compare June 3, 2024 21:18

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 3, 2024

weihanglo changed the title ~~feat(opt-dist): respect existing .cargo/config.toml~~ feat(opt-dist): new flag --benchmark-cargo-config Jun 3, 2024

Kobzol approved these changes Jun 3, 2024

View reviewed changes

weihanglo marked this pull request as ready for review June 3, 2024 21:28

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 3, 2024

compiler-errors mentioned this pull request Jun 4, 2024

Rollup of 9 pull requests #125975

Closed

GuillaumeGomez mentioned this pull request Jun 4, 2024

Rollup of 11 pull requests #125989

Merged

bors merged commit 610592b into rust-lang:master Jun 4, 2024
6 checks passed

rustbot added this to the 1.80.0 milestone Jun 4, 2024

workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Jun 5, 2024

Revert "Rollup merge of rust-lang#125930 - weihanglo:opt-dist-respect…

a1af14a

…-cargo-config, r=Kobzol" This reverts commit 610592b, reversing changes made to d9e149d.

weihanglo deleted the opt-dist-respect-cargo-config branch June 6, 2024 21:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(opt-dist): new flag `--benchmark-cargo-config` #125930

feat(opt-dist): new flag `--benchmark-cargo-config` #125930

weihanglo commented Jun 3, 2024 •

edited

Loading

rustbot commented Jun 3, 2024

rustbot commented Jun 3, 2024

weihanglo commented Jun 3, 2024 •

edited

Loading

weihanglo commented Jun 3, 2024

This comment has been minimized.

bors commented Jun 3, 2024

This comment has been minimized.

bors commented Jun 3, 2024

weihanglo commented Jun 3, 2024

This comment has been minimized.

bors commented Jun 3, 2024

bors commented Jun 3, 2024

This comment has been minimized.

Kobzol commented Jun 3, 2024

weihanglo commented Jun 3, 2024

Kobzol commented Jun 3, 2024

rust-timer commented Jun 3, 2024

weihanglo commented Jun 3, 2024

Kobzol left a comment

rustbot commented Jun 3, 2024

weihanglo commented Jun 3, 2024

bors commented Jun 3, 2024

feat(opt-dist): new flag --benchmark-cargo-config #125930

feat(opt-dist): new flag --benchmark-cargo-config #125930

Conversation

weihanglo commented Jun 3, 2024 • edited Loading

Footnotes

rustbot commented Jun 3, 2024

rustbot commented Jun 3, 2024

weihanglo commented Jun 3, 2024 • edited Loading

weihanglo commented Jun 3, 2024

This comment has been minimized.

bors commented Jun 3, 2024

This comment has been minimized.

bors commented Jun 3, 2024

weihanglo commented Jun 3, 2024

This comment has been minimized.

bors commented Jun 3, 2024

bors commented Jun 3, 2024

This comment has been minimized.

Kobzol commented Jun 3, 2024

weihanglo commented Jun 3, 2024

Kobzol commented Jun 3, 2024

rust-timer commented Jun 3, 2024

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

weihanglo commented Jun 3, 2024

Kobzol left a comment

Choose a reason for hiding this comment

rustbot commented Jun 3, 2024

weihanglo commented Jun 3, 2024

bors commented Jun 3, 2024

feat(opt-dist): new flag `--benchmark-cargo-config` #125930

feat(opt-dist): new flag `--benchmark-cargo-config` #125930

weihanglo commented Jun 3, 2024 •

edited

Loading

weihanglo commented Jun 3, 2024 •

edited

Loading