Add the start of a basic benchmarking suite.

[ehuss]

This adds the start of a basic benchmarking suite for cargo. This is fairly rough, but I figure it will change and evolve over time based on what we decide to add and how we use it.

There is some documentation in the benches/README.md file which gives an overview of what is here and how to use it.

Closes #9935

[rust-highfive]

r? @Eh2406

(rust-highfive has picked a reviewer for you, use r? to override)

[Eh2406]

Looks like this has committed a lot of files! (1571) So many that my browser crashes trying to review the change. Is this intentional? If so should we have it in repo or find some way to make it opt in?

[ehuss]

Hm, yea, that is a lot. Those are all the Cargo.toml (and lib.rs) files for the 8 sample workspaces I collected. I'll look into compressing them.

[Eh2406]

">700" of them are .rs files.

[ehuss]

Ok, I pushed an update that uses compressed workspaces. That should be much lighter, though it still adds nearly 500k of data. 😦

[Mark-Simulacrum]

Passing thought: instead of tarballs, which may end up being somewhat painful to maintain, perhaps editing the Cargo.toml's to point to a single (empty) lib.rs might make sense? It looks like across the current tarballs there are 775 .rs files, 780 .toml, and 9 .lock files -- keeping the .toml and .lock files easily accessible seems like a win to me (for understanding the benchmarks, at least), while merging all the .rs files seems both not too hard and relatively impactful - roughly 2x less files. That'll probably also shrink the .tgz files checked in here, I imagine.

FWIW with perf.rust-lang.org we initially had all the benchmarks in a separate repository but that ended up being quite painful, so we gave up on that and moved them all in-tree. (There we have much less ability to delete files, though, other than tests and such).

[ehuss]

Yea, I was reluctant to move to compressed files because it is a bit harder to see them and see changes in PRs.
I don't think it is the empty lib.rs files that really make storing them uncompressed as the problem. Although removing them would help (and would be relatively easy), that's still a huge number of Cargo.toml files which will likely be a little annoying (particularly for fuzzy filename completion in editors and such).

My thinking of why compressed files may not be too bad:

• The capture tool makes it easy to update and add new workspaces.
• I don't expect these workspaces to ever change. I can't imagine a reason to do so, and I think they are pretty sufficiently representative. If there are edge cases that aren't covered, I'd like to explore doing a more synthetic approach (like how resolver-tests uses a simple macro).
• You can view and explore the workspace pretty easily by uncompressing it (or, just go to the target/tmp/benches/workspaces directory where they get extracted).

I briefly considered having it just fetch the repos from GitHub, but things like firefox are too large even with a shallow fetch.

But yea, I agree storing compressed files is not great.

[Mark-Simulacrum]

Yeah, it's a good point that they are highly unlikely to change. I guess one question might be "should this be in rust-lang/cargo", or should we have a rust-lang/cargo-perf (like rustc-perf)? I think it probably isn't worth worrying over creating a new repository and such for the ~380 KB added in this PR, particularly as it's likely to be static data, and we're not anticipating major investment & expansion at this time. It can also be moved out in the future.

[alexcrichton]

This looks great to me, thanks for adding this! I think the capture tool is especially useful here and makes it pretty flexible into the future. I agree that it'd be good to add more here but that's all fine to have as later PRs.

I personally think it's ok to add a few tarballs here and not try to uncompress them. One thing I might add, though, is could the git commit that they were created from get added to the tarball? Ideally that'd make it easier to deterministically reproduce them in the future. If things become unwieldy otherwise we can always move them out to a separate repository or something like that.

For CI, is it possible to execute the actual benchmarks, but perhaps with just one iteration? Ideally I'd like to make sure that things don't panic at runtime in addition to at least being able to compile.

[Eh2406]

For CI, is it possible to execute the actual benchmarks, but perhaps with just one iteration?

According to the docs the incantation for that is cargo test --benches

If things become unwieldy otherwise we can always move them out to a separate repository or something like that.

Yes, but even if we do they will still be making the history bigger. Witch makes CI slower for us and rustc. Evan so I think it makes sense to have them in tree.

Theoretically, if there is a lock file then all the processing should be O(n). In practice I would not be surprised if it was more like O(n^2). The pathological cases require not having a lockfile. I look forward to a follow up PR that benches rebuilding the lockfile for these workspaces.

Over all I like this, and big thank you to @ehuss for making it!

[ehuss]

I pushed an update with a few changes:

• Runs a single iteration in CI to verify it works.
• Added git information to the workspace tgz files so they can be recreated.
• Updated the docs a bit.
• Rearranged the code so that filters can be used to prevent downloading all workspaces (like cargo bench tikv will only download the tikv workspace).

[ehuss]

This is the part I was having trouble with. I don't see a way around it, but maybe I am confused on how this all works. It seems to always do 100 samples, with only 1 iteration per sample. I kinda want the opposite, but there is a minimum of 10 samples.

[Eh2406]

I wonder if a lazy_static would work well here. Yes the first call would be slow, but that is why criterion has warm-up calls. Not shore that it would be better.

[ehuss]

Oh, good idea! I just pushed an update that actually just uses an Option to initialize it only once. That helps the benchmarks run quite a bit faster.

[ehuss]

I also disabled servo on windows, since it fails with paths that are too long.

[alexcrichton]

Looks great to me! @Eh2406 did you have anything else you wanted to add?

[Eh2406]

I want to give this a try and another look over. I hope to do so this weekend. Sorry for the delay.

[Eh2406]

Over all I love it! It does not make a big impact to checkout size, so happy to have it in tree.

A plain call to cargo bench gave me:

Benchmarking resolve_ws/tikv: Warming up for 3.0000 s    Updating git submodule `https://github.com/google/protobuf`
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: failed to resolve patches for `https://github.com/rust-lang/crates.io-index`

Caused by:
    0: failed to load source for dependency `protobuf`
    1: Unable to update https://github.com/pingcap/rust-protobuf?branch=v2.8#6642ebaa
    2: failed to update submodule `google-protobuf`
    3: path too long: 'C:/Users/SDD/documents/cargo/benches/benchsuite/target/tmp/bench/chome/git/checkouts/rust-protobuf-16c418a865b26ca8/6642eba/google-protobuf/objectivec/Tests/CocoaPods/iOSCocoaPodsTester/iOSCocoaPodsTester.xcodeproj/xcshareddata/xcschemes/iOSCocoaPodsTester.xcscheme'; class=Filesystem (30)', benches\resolve.rs:217:6
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I wonder if we could ignore patches? Or replace them with skeletons stored in the tgz? In addition to problems on windows, what happens if the git repos move?

I tried looking at a flame graph, and a bout 1/3 of time is spent in read_manifest::walk. That was something I did not know, so the benchmarks are working!

[ehuss]

A plain call to cargo bench gave me:

I added tikv to the list of workspaces to skip on Windows. Maybe when cargo supports long paths we can add it back.

I wonder if we could ignore patches? Or replace them with skeletons stored in the tgz? In addition to problems on windows, what happens if the git repos move?

I tried looking at a flame graph, and a bout 1/3 of time is spent in read_manifest::walk. That was something I did not know, so the benchmarks are working!

I had a suspicion that was why the firefox resolve was so slow. I think keeping the git dependencies is worth it to illuminate issues exactly like this. It's the fact that it is scanning an entire git repo every time that is hurting performance, and if we just used the skeletons it wouldn't be benchmarking the real-world behavior.

It might be a good idea into looking at caching the locations of packages in a git repo so it doesn't need to walk the entire tree and parse every Cargo.toml. I think that could be a very big win.

[Eh2406]

Thank you!
@bors r+

[bors]

📌 Commit e4da5b2 has been approved by Eh2406

[bors]

⌛ Testing commit e4da5b2 with merge c8b38af...

[bors]

☀️ Test successful - checks-actions
Approved by: Eh2406
Pushing c8b38af to master...