Remember names of cfg-ed out items to mention them in diagnostics

[Noratrieb]

Examples

serde::Deserialize without the derive feature (a classic beginner mistake)

I had to slightly modify serde so that it uses explicit re-exports instead of a glob re-export. (Update: a serde PR was merged that adds the manual re-exports)

error[E0433]: failed to resolve: could not find `Serialize` in `serde`
   --> src/main.rs:1:17
    |
1   | #[derive(serde::Serialize)]
    |                 ^^^^^^^^^ could not find `Serialize` in `serde`
    |
note: crate `serde` has an item named `Serialize` but it is inactive because its cfg predicate evaluated to false
   --> /home/gh-Nilstrieb/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.160/src/lib.rs:343:1
    |
343 | #[cfg(feature = "serde_derive")]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
344 | pub use serde_derive::{Deserialize, Serialize};
    |                                     ^^^^^^^^^
    = note: the item is gated behind the `serde_derive` feature
    = note: see https://doc.rust-lang.org/cargo/reference/features.html for how to activate a crate's feature

(the suggestion is not ideal but that's serde's fault)

I already tested the metadata size impact locally by compiling the windows crate without any features. 800k -> 809k

r? @ghost

[Noratrieb]

@bors try @rust-timer queue

[bors]

⌛ Trying commit c3e34e0fdd15bc1b8c94f71f8f28b84c8827e6d8 with merge c25fc4918cc5d2bf762f7443a34564369732c355...

[bors]

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

[rust-timer]

Finished benchmarking commit (c25fc4918cc5d2bf762f7443a34564369732c355): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.3%	[0.2%, 0.4%]	7
Regressions ❌ (secondary)	0.3%	[0.3%, 0.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.3%]	1
All ❌✅ (primary)	0.3%	[0.2%, 0.4%]	7

Max RSS (memory usage)

Results

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)	1.3%	[1.1%, 1.4%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.4%	[-4.6%, -1.1%]	6
All ❌✅ (primary)	1.3%	[1.1%, 1.4%]	2

Cycles

Results

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)	3.8%	[1.0%, 7.2%]	13
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.0%	[-2.0%, -2.0%]	1
All ❌✅ (primary)	3.8%	[1.0%, 7.2%]	13

[Noratrieb]

Now encodes the full item paths (as just a list of idents), which increases the metadata further (still not that much for windows), so
@bors try @rust-timer queue

[bors]

⌛ Trying commit 64f66767b681263473f54755830bb675ea93fcb4 with merge 1cc5bddfa7ccb13bce2c17c099baba2111a2dc22...

[bors]

☀️ Try build successful - checks-actions
Build commit: 1cc5bddfa7ccb13bce2c17c099baba2111a2dc22 (1cc5bddfa7ccb13bce2c17c099baba2111a2dc22)

[rust-timer]

Finished benchmarking commit (1cc5bddfa7ccb13bce2c17c099baba2111a2dc22): comparison URL.

Overall result: ❌ regressions - 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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.4%	[0.3%, 0.5%]	25
Regressions ❌ (secondary)	0.4%	[0.3%, 0.6%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.4%	[0.3%, 0.5%]	25

Max RSS (memory usage)

Results

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)	2.0%	[2.0%, 2.0%]	1
Regressions ❌ (secondary)	3.7%	[2.7%, 4.5%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.4%	[-5.1%, -2.6%]	4
All ❌✅ (primary)	2.0%	[2.0%, 2.0%]	1

Cycles

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

[Noratrieb]

perf looks good. some small regression in crates with lots of cfgs but the regressions are small enough. I also did a quick semi-scientific benchmark of windows without any features locally and the results were similar.

Benchmark 1: rm -r target && cargo +1cc5bddfa7ccb13bce2c17c099baba2111a2dc22 check
  Time (mean ± σ):     302.3 ms ±   5.7 ms    [User: 241.6 ms, System: 56.7 ms]
  Range (min … max):   291.7 ms … 317.8 ms    50 runs

Benchmark 2: rm -r target && cargo +150cb381471533050751111e5faf1d9f05c02f77 check
  Time (mean ± σ):     292.0 ms ±   5.9 ms    [User: 234.4 ms, System: 53.4 ms]
  Range (min … max):   282.1 ms … 305.8 ms    50 runs

Summary
  'rm -r target && cargo +150cb381471533050751111e5faf1d9f05c02f77 check' ran
    1.04 ± 0.03 times faster than 'rm -r target && cargo +1cc5bddfa7ccb13bce2c17c099baba2111a2dc22 check'

Sounds good to me, I'll clean it up further, add some tests and then make it ready for review.

[rust-timer]

Finished benchmarking commit (a97c36d): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.4%	[0.3%, 0.5%]	11
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.2%	[-0.2%, -0.2%]	3
Improvements ✅ (secondary)	-0.5%	[-0.6%, -0.4%]	8
All ❌✅ (primary)	0.2%	[-0.2%, 0.5%]	14

Max RSS (memory usage)

Results

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.7%	[0.7%, 0.7%]	1
Regressions ❌ (secondary)	4.5%	[2.6%, 8.0%]	3
Improvements ✅ (primary)	-1.4%	[-2.1%, -0.4%]	11
Improvements ✅ (secondary)	-2.5%	[-2.6%, -2.5%]	2
All ❌✅ (primary)	-1.2%	[-2.1%, 0.7%]	12

Cycles

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

Binary size

Results

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.6%	[0.0%, 3.0%]	60
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	8
Improvements ✅ (primary)	-0.2%	[-0.3%, -0.2%]	7
Improvements ✅ (secondary)	-0.0%	[-0.0%, -0.0%]	3
All ❌✅ (primary)	0.5%	[-0.3%, 3.0%]	67

Bootstrap: 646.353s -> 648.482s (0.33%)

[nnethercote]

@Nilstrieb: Sounds like the instruction count regressions were expected here. Any idea why the binary sizes increased? About 3% for all the libc cases, which is quite a lot, plus smaller amounts for other benchmarks.

[Noratrieb]

Binary size includes metadata, doesn't it?metadata grows a bit here as expected.

[Kobzol]

Metadata is this: https://perf.rust-lang.org/compare.html?start=b2807b2bf3be9fbed9a0aadc97841c20a1ab5f79&end=a97c36dd2e6f711949fc9b790476e93bd9e6d1f4&stat=size%3Acrate_metadata

I'm not sure if metadata can also somehow affect binary size.

[Noratrieb]

i cannot imagine how this PR would have an effect on the binary except metadata. But the regressed binary size crates nicely correlate with the icount regression crates, so I assume it includes metadata.

[pnkfelix]

• instruction counts regressed (expected) and binary sizes regressed (which may have been unexpected).
• specifically the binary sizes for libc and syn both regressed on the order of 3%.
• not marking as triaged until we get confirmation that it is expected for the metadata in question to leak into the binary sizes being measured.

[Kobzol]

The binary size metric in the dashboard also includes metadata, because most benchmarks are libraries. I didn't realize it sooner, sorry. Soon we will see in the dashboard which benchmarks are libraries/binaries, and then it should be much clearer.

[Kobzol]

@rustbot label: +perf-regression-triaged

[joshtriplett]

Does this only flag features? I tried using something that's configured out based on target and didn't get any improved errors:

/tmp/my-portable-project$ cargo +nightly build --target x86_64-pc-windows-gnu
   Compiling my-portable-project v0.1.0 (/tmp/my-portable-project)
error[E0432]: unresolved import `std::os::fd`
 --> src/main.rs:1:14
  |
1 | use std::os::fd::OwnedFd;
  |              ^^ could not find `fd` in `os`

For more information about this error, try `rustc --explain E0432`.
error: could not compile `my-portable-project` (bin "my-portable-project") due to previous error

[Noratrieb]

It should work for all cfgs, but it's not perfect. I think with imports it still doesn't work everywhere? I've opened issues somewhere but do open a new one if you don't find one already. The relevant diagnostics code is pretty messy so it's hard to catch all cases easily.

[joshtriplett]

@Nilstrieb #115185