Experimental: Add Derive Proc-Macro Caching by futile · Pull Request #129102 · rust-lang/rust (original) (raw)
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html)
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
bors added a commit to rust-lang-ci/rust that referenced this pull request
…ng, r=
Experimental: Add Derive Proc-Macro Caching
On-Disk Caching For Derive Proc-Macro Invocations
This PR adds on-disk caching for derive proc-macro invocations using rustc's query system to speed up incremental compilation.
The implementation is (intentionally) a bit rough/incomplete, as I wanted to see whether this helps with performance before fully implementing it/RFCing etc.
I did some ad-hoc performance testing.
Rough, Preliminary Eval Results:
Using a version built through DEPLOY=1 src/ci/docker/run.sh dist-x86_64-linux (which I got from here).
Some Small Personal Project:
# with -Zthreads=0 as well
$ touch src/main.rs && cargo +dist checkCaused a re-check of 1 crate (the only one).
Result:
| Configuration | Time (avg. ~5 runs) |
|---|---|
| Uncached | ~0.54s |
| Cached | ~0.54s |
No visible difference.
Bevy:
$ touch crates/bevy_ecs/src/lib.rs && cargo +dist checkCaused a re-check of 29 crates.
Result:
| Configuration | Time (avg. ~5 runs) |
|---|---|
| Uncached | ~6.4s |
| Cached | ~5.3s |
Roughly 1s, or ~17% speedup.
Polkadot-Sdk:
Basically this script (not mine): https://github.com/coderemotedotdev/rustc-profiles/blob/d61ad38c496459d82e35d8bdb0a154fbb83de903/scripts/benchmark_incremental_builds_polkadot_sdk.sh
TL;DR: Two full cargo check runs to fill the incremental caches (for cached & uncached). Then 10 repetitions of touch $some_file && cargo +uncached check && cargo +cached check.
$ cargo update # `time` didn't build because compiler too new/dep too old
$ ./benchmark_incremental_builds_polkadot_sdk.sh # see aboveHuge workspace with ~190 crates. Not sure how many were re-built/re-checkd on each invocation.
Result:
| Configuration | Time (avg. 10 runs) |
|---|---|
| Uncached | 99.4s |
| Cached | 67.5s |
Very visible speedup of 31.9s or ~32%.
-> Based on these results I think it makes sense to do a rustc-perf run and see what that reports.
Current Limitations/TODOs
I left some FIXME(pr-time)s in the code for things I wanted to bring up/draw attention to in this PR. Usually when I wasn't sure if I found a (good) solution or when I knew that there might be a better way to do something; See the diff for these.
High-Level Overview of What's Missing For "Real" Usage:
- Add caching for
Bang- andAttr-proc macros (currently onlyDerive).- Not a big change, I just focused on
derive-proc macros for now, since I felt like these should be most cacheable and are used very often in practice.
- Not a big change, I just focused on
- Allow marking specific macros as "do not cache" (currently only all-or-nothing).
- Extend the unstable option to support, e.g.,
-Z cache-derive-macros=some_pm_crate::some_derive_macro_fnfor easy testing using the nightly compiler. - After Testing: Add a
#[proc_macro_cacheable]annotation to allow proc-macro authors to "opt-in" to caching (or sth. similar). Would probably need an RFC? - Might make sense to try to combine this with rust-lang#99515, so that external dependencies can be picked up and be taken into account as well.
- Extend the unstable option to support, e.g.,
So, just since you were in the loop on the attempt to cache declarative macro expansions:
r? @petrochenkov
Please feel free to re-/unassign!
Finally: I hope this isn't too big a PR, I'll also show up in Zulip since I read that that is usually appreciated. Thanks a lot for taking a look! :)
(Kind of related/very similar approach, old declarative macro caching PR: rust-lang#128747)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…-panic, r=michaelwoerister
Prevent double panic in query system, improve diagnostics
I stumbled upon a double-panic in the query system while working on something else (rust-lang#129102), which hid the real error cause for what I was debugging. This PR remedies that, so unwinding should be able to present more errors. It shouldn't really be relevant for code that doesn't ICE.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#129271 - futile:query-system/prevent-double-panic, r=michaelwoerister
Prevent double panic in query system, improve diagnostics
I stumbled upon a double-panic in the query system while working on something else (rust-lang#129102), which hid the real error cause for what I was debugging. This PR remedies that, so unwinding should be able to present more errors. It shouldn't really be relevant for code that doesn't ICE.
