refactor: add rustc-perf submodule to src/tools

[lovesegfault]

Currently, it's very challenging to perform a sandboxed opt-dist
bootstrap because the tool requires rustc-perf to be present, but
there is no proper management/tracking of it. Instead, a specific commit
is hardcoded where it is needed, and a non-checksummed zip is fetched
ad-hoc. This happens in two places:

src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile:

ENV PERF_COMMIT 4f313add609f43e928e98132358e8426ed3969ae
RUN curl -LS -o perf.zip https://ci-mirrors.rust-lang.org/rustc/rustc-perf-$PERF_COMMIT.zip && \
    unzip perf.zip && \
    mv rustc-perf-$PERF_COMMIT rustc-perf && \
    rm perf.zip

src/tools/opt-dist/src/main.rs

// FIXME: add some mechanism for synchronization of this commit SHA with
// Linux (which builds rustc-perf in a Dockerfile)
// rustc-perf version from 2023-10-22
const PERF_COMMIT: &str = "4f313add609f43e928e98132358e8426ed3969ae";

let url = format!("https://ci-mirrors.rust-lang.org/rustc/rustc-perf-{PERF_COMMIT}.zip");
let client = reqwest::blocking::Client::builder()
    .timeout(Duration::from_secs(60 * 2))
    .connect_timeout(Duration::from_secs(60 * 2))
    .build()?;
let response = retry_action(
    || Ok(client.get(&url).send()?.error_for_status()?.bytes()?.to_vec()),
    "Download rustc-perf archive",
    5,
)?;

This causes a few issues:

1. Maintainers need to be careful to bump PERF_COMMIT in both places
   every time
2. In order to run opt-dist in a sandbox, you need to provide your own
   rustc-perf (feat(tools/opt-dist): allow local builds to specify a rustc-perf checkout #125125), but to
   figure out which commit to provide you need to grep the Dockerfile
3. Even if you manage to provide the correct rustc-perf, its
   dependencies are not included in the vendor/ dir created during
   dist, so it will fail to build from the published source tarballs
4. It is hard to provide any level of automation around updating the
   rustc-perf in use, leading to staleness

Fundamentally, this means rustc-src tarballs no longer contain
everything you need to bootstrap Rust, and packagers hoping to leverage
opt-dist need to go out of their way to keep track of this "hidden"
dependency on rustc-perf.

This change adds rustc-perf as a git submodule, pinned to the current
PERF_COMMIT 4f313ad. Subsequent
commits ensure the submodule is initialized when necessary, and make use
of it in opt-dist.

[rustbot]

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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]

⚠️ Warning ⚠️

• These commits modify submodules.

[Kobzol]

This is an interesting idea! How long does it take to checkout rustc-perf and how large is it (in vendored/dist archives)? It contains a lot of code.

[Kobzol]

I'm not sure if we should do this unconditionally, since opt-dist is only used in two CI dist jobs, most of them don't use it at all 🤔

[lovesegfault]

since opt-dist is only used in two CI dist jobs, most of them don't use it at all

That's true within the Rust project, but my main goal here is to make opt-dist usable for packagers / end users who need to bootstrap Rust themselves, but still want to benefit from PGO and BOLT.

[lovesegfault]

I'm not sure if we should do this unconditionally

I wish we had a "src tarball for developers", which would not include any submodules for example, and a "src tarball for distributing" which would be "complete", but that would be a pretty large yak to shave.

[Nilstrieb]

Which "developers" do you think are using it?

[lovesegfault]

Which "developers" do you think are using it?

I think few people use it today, but that's likely because of how hard it is to leverage opt-dist outside the Rust CI. I think, in the future, it would be used:

• Directly by anyone who wants to package Rust, needs to bootstrap it themselves, and wants to build an optimized dist with PGO+BOLT. Namely, large distros and companies would benefit from this.
• Indirectly by anyone consuming Rust from the above distributors. Namely, folks using Rust from their package manager, and people using Rust at large companies.

[lovesegfault]

For future reference, my argument boils down to:

premises

• the point of the rustc-dev-src dist tarball is to allow people to bootstrap Rust
• most people bootstrapping Rust want a production-grade toolchain as a result
• the vanilla bootstrap is not sufficient to bootstrap a production-grade rust-toolchain
• it is undesirable for users to be forced to reinvent the wheel and roll their own complex PGO/BOLT bootstrapping pipeline

therefore

• it should be possible to bootstrap a production-grade Rust toolchain from the rustc-dev-src tarball using opt-dist

[lovesegfault]

@Kobzol

How long does it take to checkout rustc-perf

I'd be happy to measure this, but I feel doing so on my machine will not be representative. I could use something like trickle to artificially limit bandwidth. Is there a particular bandwidth target you feel is fair?

how large is it (in vendored/dist archives)?

That's my main concern. I submitted the PR while waiting for the build of master to finish locally to compare, here's the result:

 Size Name
415Mi rustc-1.80.0-dev-src-master.tar.gz
606Mi rustc-1.80.0-dev-src-with-rustc-perf.tar.gz
~46% larger 

231Mi rustc-1.80.0-dev-src-master.tar.xz
318Mi rustc-1.80.0-dev-src-with-rustc-perf.tar.xz
~37% larger

It's not great.

I wonder whether it'd be possible to not use all of rustc-perf to build with PGO/BOLT.

[Mark-Simulacrum]

The source tarball here is only really used by distros or other downstream builders, right? I feel like the size of it doesn't matter that much...

[rustbot]

The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging.

cc @davidtwco, @wesleywiser

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

[lqd]

I wonder whether it'd be possible to not use all of rustc-perf to build with PGO/BOLT.

Yes, we don’t use a lot from rustc-perf actually: not all the compile time benchmarks, the runtime benchmarks, the website or its dependencies, and basically only use the collector like a script to run rustc over the chosen benchmarks, without doing any DB/network perf collection and these dependencies.

[lovesegfault]

Yes, we don’t use a lot from rustc-perf actually: not all the compile time benchmarks, the runtime benchmarks, the website or its dependencies, and basically only use the collector like a script to run rustc over the chosen benchmarks, without doing any DB/network perf collection and these dependencies.

Nice, while I don't think it should block this PR, I wonder if it would make sense to either:

1. Split the rustc-perf repo into rustc-perf-collector, rustc-perf-corpus, and rustc-perf, and then change this logic in the future to only pick up the collector and corpus.
2. Split the rustc-perf collector into its own repo and create a separate corpus for opt-dist

@Kobzol, thoughts? I don't know how de/coupled the collector/corpus/website are.

[Kobzol]

They are very coupled, and amongst other things the website depends on code from collector. I would very much like to avoid any splitting of the repository into more repos, that would make working on it very painful (if anything, I would like to merge repos into monorepos, if possible :) ).

I guess that we could create a branch that would only contain the collector directory, but I'm not sure if it's worth it.

[Kobzol]

Although it was not really an original goal of opt-dist, making it easier for distro maintainers to perform an optimized build is definitely a non-trivial benefit. If the src archives are anyway mostly only used by distro maintainers and similar downstream users, probably increasing its size is not such a big deal, as Mark said.

So I guess that it's fine to just vendor everything. What I was more concerned about was the regular development workflow for people hacking on rustc, that's why I was asking about the checkout speed (something like 50 Mbit/s might be reasonable to simulate?) But we have llvm-project as a submodule, so rustc-perf probably won't be the worst offender in this regard :)

[Nilstrieb]

But we have llvm-project as a submodule,

people working on this repo often don't have that checked out though

[lovesegfault]

What I was more concerned about was the regular development workflow for people hacking on rustc, that's why I was asking about the checkout speed (something like 50 Mbit/s might be reasonable to simulate?)

As-is, the submodule is only initialized/updated when either:

1. You build opt-dist
2. You build the dist target

I don't think developer workflows will be impacted by this change at all, unless you're specifically doing a lot of clean dist bootstraps, in which case the submodule fetch is not really going to be a significant portion of the overall build time.

I was careful to not include the rustc-perf submodule in other update_submodule calls.

[Kobzol]

people working on this repo often don't have that checked out though

Oh, I thought that it happens by default. Well, as @lovesegfault said, rustc-perf would only be checked when you build opt-dist.

I'm still not sure if we need to checkout the submodule and (primarily) vendor rustc-perf when we do dist for platforms other han Linux x86 (well, and technically Windows x64, but I kind of doubt that any "Windows x64 distro maintainers" need PGO/BOLT builds :) ) though. On other platforms, opt-dist is not really applicable at the moment.

[lovesegfault]

On other platforms, opt-dist is not really applicable at the moment.

For what it's worth, you can leverage opt-dist to bootstrap Rust with PGO on aarch64-unknown-linux-gnu today, and it works, though you're right the project isn't doing this yet.

BOLT, on the other hand, really only works on x86_64 for now.

[lovesegfault]

Gathered data on cloning rustc-perf:

$ hyperfine --warmup 2 --prepare 'rm -rf ./rustc-perf' 'trickle -s -d 50000 git clone https://github.com/rust-lang/rustc-perf'
Benchmark 1: trickle -s -d 50000 git clone https://github.com/rust-lang/rustc-perf
  Time (mean ± σ):      3.027 s ±  0.029 s    [User: 3.181 s, System: 2.307 s]
  Range (min … max):    2.988 s …  3.079 s    10 runs

$ hyperfine --warmup 2 --prepare 'rm -rf ./rustc-perf' 'trickle -s -d 25000 git clone https://github.com/rust-lang/rustc-perf'
Benchmark 1: trickle -s -d 25000 git clone https://github.com/rust-lang/rustc-perf
  Time (mean ± σ):      4.341 s ±  0.022 s    [User: 3.668 s, System: 3.521 s]
  Range (min … max):    4.312 s …  4.372 s    10 runs

$ hyperfine --warmup 2 --prepare 'rm -rf ./rustc-perf' 'trickle -s -d 10000 git clone https://github.com/rust-lang/rustc-perf'
Benchmark 1: trickle -s -d 10000 git clone https://github.com/rust-lang/rustc-perf
  Time (mean ± σ):      8.535 s ±  0.037 s    [User: 4.533 s, System: 6.928 s]
  Range (min … max):    8.485 s …  8.604 s    10 runs

[lqd]

Another datapoint: a computer I was using to analyze a windows-gnu bug takes 40s to do that git clone 😄

[Kobzol]

In the grand scheme of things, cloning the submodule will probably be fine, since it's optional. I'm generally in favour of this change, it resolves a minor maintenance annoyance (keeping the rustc-perf version in sync), and could help more distro maintainers get perf. benefits for their Rust toolchain.

Let's see if it works in the current state.

@bors try @rust-timer queue

[bors]

⌛ Trying commit 83a847f with merge bf9ad42...

[Mark-Simulacrum]

Why are we no longer tracking the deps for opt-dist? I see that there's a bunch of dependencies that disappeared from our lockfile as well -- where are those being locked? I don't see a new Cargo.lock added here...

[lovesegfault]

We were able to remove those dependencies from opt-dist, since it no longer needs to fetch anything over the network.

I removed those lines because, IIRC, tidy complained that they were no longer reflected in the Cargo.lock once I removed the deps from opt-dist, which made sense.

[Mark-Simulacrum]

Ah, perfect. I missed that part of the diff in my skim I guess :)

Cutting some dependencies is great.

[bors]

☔ The latest upstream changes (presumably #125313) made this pull request unmergeable. Please resolve the merge conflicts.

[Mark-Simulacrum]

@bors r+

[bors]

📌 Commit e253718 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

[bors]

⌛ Testing commit e253718 with merge 60faa27...

[bors]

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 60faa27 to master...

[rust-timer]

Finished benchmarking commit (60faa27): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (primary -0.6%, secondary -2.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)	2.5%	[2.5%, 2.5%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.1%	[-2.3%, -1.9%]	2
Improvements ✅ (secondary)	-2.4%	[-2.4%, -2.4%]	1
All ❌✅ (primary)	-0.6%	[-2.3%, 2.5%]	3

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: 670.311s -> 675.351s (0.75%)
Artifact size: 315.45 MiB -> 315.41 MiB (-0.01%)