[PERF Experiment] Only create the self-profile infra if the profiler is actually enabled (only if -Z self-profile)

[blyxyas]

Previously, we would use the Session::time and this would just call verbose_generic_activity(...), which would create an activity and all that... But all this effort would go unnoticed if the user didn't use the -Z self-profile feature.

So, this PR checks for the profiler before trying to profile. From my benchmarks on a server, this should be a great performance improvement (Note: I tested this with a 12-thread custom multithreaded rustc-perf clone to mimick a compiler that a user might use, the results may change by using the single-threaded rustc-perf that bors currently uses)

[rustbot]

r? @compiler-errors

(rustbot has picked a reviewer for you, use r? to override)

[blyxyas]

Here are the results in my independent server (with multithreading):
Regressions: [+0.42%, +4.66%] Mean: +3.13% // 3 regressive benchmarks (all secondary)
Improvements: [-9.40%, -0.38%] Mean: -2.67% // 133 improved benchmarks
Total: [-9.40%, +4.66%] Mean: -2.55% // 136 total benchmarks

Let's check on the official infrastructure (^..^)ﾉ

@bors try @rust-timer queue

[bors]

⌛ Trying commit 97814dd with merge 1ea7b41...

[blyxyas]

I... I forgot tidy

@bors try @rust-timer queue

[bors]

⌛ Trying commit 0939b86 with merge 90adef6...

[Kobzol]

I... I forgot tidy

I think that tidy isn't checked on try builds.

[Noratrieb]

#120093 (comment)

[bors]

☀️ Try build successful - checks-actions
Build commit: 90adef6 (90adef6548341c0df7114d43bcb0c2c1916a518f)

[rust-timer]

Finished benchmarking commit (90adef6): 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

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.7%	[1.7%, 1.7%]	1
Regressions ❌ (secondary)	2.6%	[1.8%, 3.5%]	8
Improvements ✅ (primary)	-4.0%	[-4.0%, -4.0%]	1
Improvements ✅ (secondary)	-1.2%	[-1.2%, -1.2%]	1
All ❌✅ (primary)	-1.2%	[-4.0%, 1.7%]	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)	-	-	0
Regressions ❌ (secondary)	3.3%	[3.3%, 3.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 660.078s -> 660.948s (0.13%)
Artifact size: 308.02 MiB -> 308.06 MiB (0.01%)

[Kobzol]

I'm not sure if it's the case here, but note in general that local benchmarks can be quite misleading, since the configuration on the server is quite different from the default config.toml. To have more accurate numbers, one needs to enable jemalloc, cgu = 1, incremental = false, etc. And even then, PGO and BOLT will be missing.

Here are the results in my independent server (with multithreading):
Regressions: [+0.42%, +4.66%] Mean: +3.13% // 3 regressive benchmarks (all secondary)
Improvements: [-9.40%, -0.38%] Mean: -2.67% // 133 improved benchmarks
Total: [-9.40%, +4.66%] Mean: -2.55% // 136 total benchmarks

These are instruction count or wall-time results?

[compiler-errors]

r? @Kobzol

[blyxyas]

Those results are instruction counts, I'll re-benchmark with these new configuration options. It seems really weird to me that no changes are perceptible. -6% instead of -9%? That would be reasonable, but a difference no greater than 0.2%? That's very fishy

[Kobzol]

It can be a bit tricky to get stable perf. results locally, especially if incremental is enabled. I would be surprised if this change could have such a large effect. You could try running cachegrind locally to see if there's any large difference in instructions.

[blyxyas]

I've verified results with those optimizations, still some good results (General: ❌,✅ [ -9.08%, +7.10%] -2.45% 129 (38)) this should work 🤔

I'll try with cachegrind and schedule a benchmark for both Master and this branch on a local Raspberry Pi (ARM architecture, this would eliminate any load that the server I was using may have had, + it will reveal results for the ARM architecture)

[Kobzol]

I ran cachegrind locally and there is almost no or only a very small difference in instruction counts. Tbh, I don't think that this help should help much, there's not that many profiling calls in the compiler, and this only saves a few instructions per profiling call (if the profiler is disabled, it basically just does a few no-op operations on None options).

Note that this function does more than just record -Zself-profile events. It also works with -Ztime-passes, and in that case profiler can be None, but print_verbose_generic_activities can be Some. So the proposed change would break that workflow.

Btw, if you're using a custom version of rustc-perf, I'd suggest to "calibrate" it first, by compiling two versions of the compiler with a trivial change (e.g. add a comment or change some unimportant string value somewhere), to see if the baseline will indeed be near zero difference in instruciton counts.

[blyxyas]

Okis, it's callibrated and the two results in the control benchmarks are the same (except for when I accidentally entered the SSH after a few failed passwords, that made a +0.40% change on a single benchmark), but as you said, it isn't a big change, so we can't expect a big improvement.

I'll learn to use cachegrind, and if I don't see any performance improvement, I'll just let this patch go and focus on the next thing. Although maybe the infra server uses -Z self-profile, and my server uses perf, in which case the infra server wouldn't see any difference

[Kobzol]

It should only use -Zself-profile for a single run within the benchmark collection, and the results from that run are not stored in the database anymore. The rest of the runs should be without self profiling.