Cache derive proc macro expansion with incremental query

[Kobzol]

This is a revival of #129102, originally implemented by @futile. Since it looks like they are not active currently, I'd like to push this work forward.

The first commit is squashed and rebased work from the original PR, with author attribution to futile. The rest of the commits are some additional comments that I created mostly for myself to understand what happens here. I also did some cleanups based on Vadim's review comments on the original PR, plus I refactored the TLS access a bit using scoped_tls.

The biggest issue, as usually, are tests... I tried using #[rustc_clean(..., loaded_from_disk = "derive_macro_expansion")], but the problem is that since this query cannot recover the original key from its hash, and thus its fingerprintstyle is FingerprintStyle::Opaque, this crashes when I try to use loaded_from_disk. Any suggestions from someone who actually understands the query system would be welcome 😅

You can try it yourself on nightly with RUSTFLAGS="-Zcache-proc-macros" cargo build.

On a no-op change re-check of octocrab 0.49 (which has a ton of serde derive proc macro invocations), this saves ~0.6s out of ~6s (so a ~10% win) on my PC.

r? @petrochenkov

[rustbot]

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.

[Kobzol]

@bors try @rust-timer queue

[Kobzol]

@bors try

[Kobzol]

I looked into the benchmark failure on diesel, seems like I hold queries wrong somehow :) Any suggestions on what could be the case? Maybe the derived Hash impl for TokenStream does not correspond to the manual Eq impl? 🤔

thread 'rustc' panicked at /projects/personal/rust/rust/compiler/rustc_query_system/src/query/plumbing.rs:191:23:
active query job entry
stack backtrace:
   0: __rustc::rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::option::expect_failed
   3: <rustc_query_system::query::plumbing::JobOwner<(rustc_span::hygiene::LocalExpnId, rustc_data_structures::svh::Svh, &rustc_ast::tokenstream::TokenStream), rustc_query_system::query::QueryStackDeferred>>::complete::<rustc_query_system::query::caches::DefaultCache<(rustc_span::hygiene::LocalExpnId, rustc_data_structures::svh::Svh, &rustc_ast::tokenstream::TokenStream), rustc_middle::query::erase::Erased<[u8; 8]>>>
   4: rustc_query_system::query::plumbing::try_execute_query::<rustc_query_impl::DynamicConfig<rustc_query_system::query::caches::DefaultCache<(rustc_span::hygiene::LocalExpnId, rustc_data_structures::svh::Svh, &rustc_ast::tokenstream::TokenStream), rustc_middle::query::erase::Erased<[u8; 8]>>, false, false, false>, rustc_query_impl::plumbing::QueryCtxt, true>
   5: <scoped_tls::ScopedKey<rustc_expand::proc_macro::QueryDeriveExpandCtx>>::set::<<rustc_expand::proc_macro::QueryDeriveExpandCtx>::enter<<rustc_expand::proc_macro::DeriveProcMacro as rustc_expand::base::MultiItemModifier>::expand::{closure#1}::{closure#0}, core::result::Result<rustc_ast::tokenstream::TokenStream, ()>>::{closure#0}, core::result::Result<rustc_ast::tokenstream::TokenStream, ()>>
   6: <rustc_expand::proc_macro::QueryDeriveExpandCtx>::enter::<<rustc_expand::proc_macro::DeriveProcMacro as rustc_expand::base::MultiItemModifier>::expand::{closure#1}::{closure#0}, core::result::Result<rustc_ast::tokenstream::TokenStream, ()>>
   7: <rustc_expand::proc_macro::DeriveProcMacro as rustc_expand::base::MultiItemModifier>::expand
   8: <rustc_expand::expand::MacroExpander>::expand_invoc
   9: <rustc_expand::expand::MacroExpander>::fully_expand_fragment
  10: <rustc_expand::expand::MacroExpander>::expand_crate
  11: <rustc_session::session::Session>::time::<rustc_ast::ast::Crate, rustc_interface::passes::configure_and_expand::{closure#1}>
  12: rustc_interface::passes::resolver_for_lowering_raw
      [... omitted 6 frames ...]
  13: <rustc_middle::ty::context::TyCtxt>::resolver_for_lowering
  14: <std::thread::local::LocalKey<core::cell::Cell<*const ()>>>::with::<rustc_middle::ty::context::tls::enter_context<<rustc_middle::ty::context::GlobalCtxt>::enter<rustc_interface::passes::create_and_enter_global_ctxt<core::option::Option<rustc_interface::queries::Linker>, rustc_driver_impl::run_compiler::{closure#0}::{closure#2}>::{closure#2}::{closure#0}, core::option::Option<rustc_interface::queries::Linker>>::{closure#1}, core::option::Option<rustc_interface::queries::Linker>>::{closure#0}, core::option::Option<rustc_interface::queries::Linker>>
  15: <rustc_middle::ty::context::TyCtxt>::create_global_ctxt::<core::option::Option<rustc_interface::queries::Linker>, rustc_interface::passes::create_and_enter_global_ctxt<core::option::Option<rustc_interface::queries::Linker>, rustc_driver_impl::run_compiler::{closure#0}::{closure#2}>::{closure#2}::{closure#0}>
  16: <rustc_interface::passes::create_and_enter_global_ctxt<core::option::Option<rustc_interface::queries::Linker>, rustc_driver_impl::run_compiler::{closure#0}::{closure#2}>::{closure#2} as core::ops::function::FnOnce<(&rustc_session::session::Session, rustc_middle::ty::context::CurrentGcx, alloc::sync::Arc<rustc_data_structures::jobserver::Proxy>, &std::sync::once_lock::OnceLock<rustc_middle::ty::context::GlobalCtxt>, &rustc_data_structures::sync::worker_local::WorkerLocal<rustc_middle::arena::Arena>, &rustc_data_structures::sync::worker_local::WorkerLocal<rustc_hir::Arena>, rustc_driver_impl::run_compiler::{closure#0}::{closure#2})>>::call_once::{shim:vtable#0}
  17: <alloc::boxed::Box<dyn for<'a> core::ops::function::FnOnce<(&'a rustc_session::session::Session, rustc_middle::ty::context::CurrentGcx, alloc::sync::Arc<rustc_data_structures::jobserver::Proxy>, &'a std::sync::once_lock::OnceLock<rustc_middle::ty::context::GlobalCtxt<'a>>, &'a rustc_data_structures::sync::worker_local::WorkerLocal<rustc_middle::arena::Arena<'a>>, &'a rustc_data_structures::sync::worker_local::WorkerLocal<rustc_hir::Arena<'a>>, rustc_driver_impl::run_compiler::{closure#0}::{closure#2}), Output = core::option::Option<rustc_interface::queries::Linker>>> as core::ops::function::FnOnce<(&rustc_session::session::Session, rustc_middle::ty::context::CurrentGcx, alloc::sync::Arc<rustc_data_structures::jobserver::Proxy>, &std::sync::once_lock::OnceLock<rustc_middle::ty::context::GlobalCtxt>, &rustc_data_structures::sync::worker_local::WorkerLocal<rustc_middle::arena::Arena>, &rustc_data_structures::sync::worker_local::WorkerLocal<rustc_hir::Arena>, rustc_driver_impl::run_compiler::{closure#0}::{closure#2})>>::call_once
  18: rustc_interface::passes::create_and_enter_global_ctxt::<core::option::Option<rustc_interface::queries::Linker>, rustc_driver_impl::run_compiler::{closure#0}::{closure#2}>
  19: std::panic::catch_unwind::<core::panic::unwind_safe::AssertUnwindSafe<rustc_interface::interface::run_compiler<(), rustc_driver_impl::run_compiler::{closure#0}>::{closure#1}::{closure#0}>, ()>
  20: <scoped_tls::ScopedKey<rustc_span::SessionGlobals>>::set::<rustc_interface::util::run_in_thread_with_globals<rustc_interface::util::run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<(), rustc_driver_impl::run_compiler::{closure#0}>::{closure#1}, ()>::{closure#0}, ()>::{closure#0}::{closure#0}::{closure#0}, ()>
  21: rustc_span::create_session_globals_then::<(), rustc_interface::util::run_in_thread_with_globals<rustc_interface::util::run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<(), rustc_driver_impl::run_compiler::{closure#0}>::{closure#1}, ()>::{closure#0}, ()>::{closure#0}::{closure#0}::{closure#0}>
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace

[Kobzol]

Ok I did a bit of debugging, and found a fun fact: TokenStream == TokenStream is (or at least can be) false. So that has to be fixed before we can use token streams as query keys.. :)

[Kobzol]

Ok I found it, it's this. @nnethercote Do you think it's possible to make TokenStreams comparable?

[petrochenkov]

Technically, removing that PartialEq for InvisibleOrigin is a language change.
But this PR can be unblocked without the lang team involvement by migrating the macro_rules matching logic specifically from PartialEq to a custom comparison function.

I'm not actually sure where else PartialEq for TokenStream is used, besides the matching and now also queries.
UPD: Nowhere.
UPD2: I think (Partial)Eq for TokenStream can also be derived now.

[Kobzol]

Could you please specify what do you mean by "macro rules matching logic"? I tried removing PartialEq from InvisibleOrigin, which required removing it from Delimiter and TokenKind, and that makes a bunch of places in code not compile, because they use == on these types (approx. 13 usages across 4 files). All of that should be migrated to a custom trait?

I also thought that another alternative would be to wrap TokenStream in a newtype and implement Eq for that newtype, which would delegate to a custom comparison trait for the query key logic (so basically an inverse of what you proposed).

[petrochenkov]

CI is also failing.
@rustbot author

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[Kobzol]

@rustbot ready

[petrochenkov]

r=me after addressing the remaining nits and squashing commits.
@rustbot author

[Kobzol]

@bors r=petrochenkov

[rust-bors]

📌 Commit 8fa2f69 has been approved by petrochenkov

It is now in the queue for this repository.

[futile]

Congratz @Kobzol, and thanks for seeing this through! The final code looks super nice. And thanks to @petrochenkov for all the reviews :) Looking forward to updating to the next nightly 😀