-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proc macros should limit symbol exports #99909
Comments
I just about almost opened my own issue, as this looks like it could cause e.g. #59998: $ nm -C -D target/debug/deps/libserde_derive-eb8144d61a68af88.so | rg PANIC_
00000000005d31b8 B std::panicking::panic_count::GLOBAL_PANIC_COUNT (noticed the excess of exports in @fasterthanlime's latest blog post, about RA/proc macros) Frankly I think I got confused at some point and expected proc macros to get more And there are certainly places where it looks like that was the intention: rust/compiler/rustc_codegen_ssa/src/back/symbol_export.rs Lines 23 to 26 in b4151a4
rust/compiler/rustc_codegen_ssa/src/back/symbol_export.rs Lines 142 to 146 in b4151a4
But also this seems to apply? So what's the actual crate type for a proc macro? rust/compiler/rustc_codegen_ssa/src/back/symbol_export.rs Lines 260 to 267 in b4151a4
Ah there it is! For some reason we just completely bypass symbol-hiding for proc macros: rust/compiler/rustc_codegen_ssa/src/back/linker.rs Lines 651 to 661 in 9067d52
|
That is exactly what I was reading when I opened this issue :) |
(i.e. the original proc macro impl) is what appears to have added the proc macro case here (which seems to have been called rust/src/librustc_trans/back/linker.rs Lines 240 to 248 in 7a26aec
So what happened was that proc macros were a lot more like However, at some point, @michaelwoerister refactored
But that decision couldn't have changed anything (at least not in the way we might expect) because of the bail-out from export hiding that proc macros have been hit by. Nowadays not even I believe this also tripped me up in the past, since |
Removing the gate at rust/compiler/rustc_codegen_ssa/src/back/linker.rs Lines 651 to 661 in 9067d52
Edit: Symbol visibility for |
Opened #99944 with a fix. |
$ rustup update nightly
info: syncing channel updates for 'nightly-x86_64-unknown-linux-gnu'
info: latest update on 2022-08-02, rust version 1.64.0-nightly (fe3342816 2022-08-01)
...
nightly-x86_64-unknown-linux-gnu updated - rustc 1.64.0-nightly (fe3342816 2022-08-01) (from rustc 1.64.0-nightly (9067d5277 2022-07-28))
$ cargo build -p serde_derive
$ nm -D --defined-only -S target/debug/deps/libserde_derive-*.so
0000000000315f10 0000000000000010 D __rustc_proc_macro_decls_284a6d00fbf74d9e__
0000000000000000 0000000000008ca2 N rust_metadata_serde_derive_284a6d00fbf74d9e Beautiful, thanks @bjorn3! ✨ |
I wonder what the impact of this reduction has been. |
#99944 (comment) shows quite significant build time improvements. serde_derive debug got almost 3MB smaller (down to 23MB). Same for serde_derive opt (down to 8MB). I don't think we benchmark compilation of any other proc macro on perf.rust-lang.org. |
readelf --wide --syms build/x86_64-unknown-linux-gnu/stage0-rustc/release/deps/libproc_macro_hack-cdb4209108fe6fa9.so | grep GLOBAL | grep -v UND | wc -l
shows over 5500 exported symbols. It should limit exports to only those that are used, just like for cdylib's. This should decrease the size of proc macros by allowing more effecting garbage collection of dead code.The text was updated successfully, but these errors were encountered: