Store metadata in import libraries for MSVC

[Diggsey]

This reduces the size of rust dylibs by an order of magnitude in some cases.

[rust-highfive]

r? @arielb1

(rust_highfive has picked a reviewer for you, use r? to override)

[arielb1]

r? @alexcrichton

[bors]

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

[alexcrichton]

This looks basically equivalent to the rlib branch above, can these branches be shared?

[Diggsey]

Yeah, will do that

[alexcrichton]

Would it be possible to emit the symbol with a visibility level that we don't have to do this?

[alexcrichton]

Or otherwise, can you comment as to why we're doing this?

[alexcrichton]

cc @vadimcn

This looks pretty good to me, but ideally we'd unify code paths as much as possible to avoid too much divergence between platforms. It seemed a little odd to me to be frequently frobbing between .dll.rlib and .dll.lib, would it be possible to avoid those transition costs? Similarly, could you add in documentation somewhere the high level overview here? (I had to just reverse engineer what a .dll.rlib was)

[retep998]

Does the metadata really have to be removed from the .dll.rlib before it can be passed to the linker?

[Diggsey]

I'll add more of a description in the comments, but for anyone reading, the new .dll.rlibs are to .dll.libs as .rlibs are to .libs (ie. they are the same import library generated for the dll, but contain an additional object storing the rust metadata).

WRT the back and forth between .dll.lib and .dll.rlib: when producing an import library, there doesn't appear to be a way to include the metadata as part of the build, so the .dll.lib -> .dll.rlib conversion step is necessary.

For the reverse direction, I tested both with and without the code to remove the metadata prior to linking. While not removing the metadata seemed to work™️, I wasn't confident enough that there wasn't some edge case where it would fail, say under different linker flags. It's also what we do for rlibs, and I don't know what's different here that would mean that rlibs must have their metadata removed before linking, while import libraries need not?

[vadimcn]

Sounds like we are doing a bunch of unnecessary I/O every time we are linking. All for the sake of not having an extra file around?

@alexcrichton: IMHO, we should just bite the bullet and move metadata to a sidecar file for all library formats on all platforms.

[retep998]

@Diggsey If not removing the metadata before linking works, and the resulting binary doesn't suddenly become bigger, then we should definitely not waste time removing the metadata until someone can demonstrate an edge case where it does fail.

[alexcrichton]

@Diggsey I'm not sure what you mean about rlibs, we don't store metadata as an object file in there? We take it out manually when we pass -Wl,--whole-archive to the linker (which we only do some of the time) because otherwise ld will error on the unknown format.

@vadimcn do you feel that should block this PR? Or happen in parallel?

[vadimcn]

@alexcrichton: I am not super-opposed to landing this, but opposed nevertheless - on the grounds that every special case we add becomes an extra bit maintenance burden later. It is already hard to follow all the special cases we have in link.rs

[alexcrichton]

@vadimcn yeah that's true. @Diggsey can you elaborate on the technical rationale for doing this on Windows?

@vadimcn I think we'd want to consider such a change carefully though as it'd be changing the binary layout of other platforms which is susceptible to cause breakage. Whereas with windows there's already an extra file to place metadata into it seems there's a much lower risk of breaking things (although I'd prefer to avoid changing the name if possible)

[Diggsey]

@alexcrichton

I'm not sure what you mean about rlibs, we don't store metadata as an object file in there? We take it out manually when we pass -Wl,--whole-archive to the linker (which we only do some of the time) because otherwise ld will error on the unknown format.

I simply meant that the metadata was stored as an additional file within the archive, not that it was a valid object file.

I think my confusion stems from the fact that there is no --whole-archive option used on MSVC, and yet we still remove the metadata from any rlibs when producing a dylib. If it's not necessary to remove the metadata, then we shouldn't need to remove it when producing a dylib either.

It might be better if the Linker trait operated at a slightly higher level, because at the moment it's providing a very leaky abstraction.

can you elaborate on the technical rationale for doing this on Windows?

I explained this in the RFC, but to summarise:

• Space saving when distributing rust applications, including the rust compiler itself (eg. the libstd DLL is almost entirely metadata)
• Metadata is only needed at build time
• The MSVC toolchain already has a file for storing metadata about a DLL that's only needed at build time: an import library.
• Rustc already knows how to embed/extract metadata into/from a static library.

[alexcrichton]

I agree with @vadimcn that more platform-specific code means more platform-specific code, introducing more vectors for bugs and such. I also agree though that this is the "proper" solution to the problem on MSVC, however. I don't think the technical benefits here are really all that much, almost no one is shipping Rust dylibs much less on MSVC.

I'd still personally be ok landing this PR, however. (with comments addressed)

[Diggsey]

Cool, I'll make the changes ASAP.

(Also, you are shipping rust dylibs on MSVC 😉 )

[Diggsey]

Updated and squashed

[alexcrichton]

I'm a little unsure about how to proceed with this PR. I agree with @vadimcn that all the extra casing in an already very complicated backend isn't great. The technical benefits seem slim to only improving file size of the dll itself, but my guess is that this PR as-is actually inflates the size of the rustc distribution at least (due to metadata being stored uncompressed in import libraries.

I think it'd be a slam dunk, though, if we had some concrete numbers showing something like the compiler loads X% faster or we save Y bytes in the distribution. @Diggsey you wouldn't happen to have any of those numbers would you?

[retep998]

due to metadata being stored uncompressed in import libraries.

If metadata is stored compressed in the dll originally, is there any reason we can't store it compressed in the import library?

[alexcrichton]

Not particularly, no. It's not what this PR does, nor is it what we do on any other platform. I don't think it would help the "this is just adding more special cases" counterargument as well.

[Diggsey]

@alexcrichton Here is a list of DLL sizes and the changes:

Name Before After Saved
---- ------ ----- -----
arena-*.dll 103424 83456 19%
flate-*.dll 149504 143360 4%
fmt_macros-*.dll 120320 103936 14%
getopts-*.dll 178176 139776 22%
graphviz-*.dll 136192 102912 24%
log-*.dll 138752 118272 15%
proc_macro_plugin-*.dll 340992 331264 3%
proc_macro-*.dll 135168 125952 7%
rustc_back-*.dll 717312 643584 10%
rustc_borrowck-*.dll 969728 714240 26%
rustc_const_eval-*.dll 556544 446464 20%
rustc_const_math-*.dll 200192 131072 35%
rustc_data_structures-*.dll 392704 131072 67%
rustc_driver-*.dll 2099200 1955328 7%
rustc_errors-*.dll 366080 260608 29%
rustc_incremental-*.dll 830464 648704 22%
rustc_lint-*.dll 368128 325120 12%
rustc_llvm-*.dll 30444544 30369280 0%
rustc_metadata-*.dll 1357312 1097728 19%
rustc_mir-*.dll 1043456 802304 23%
rustc_passes-*.dll 401920 361472 10%
rustc_platform_intrinsics-*.dll 354816 314368 11%
rustc_plugin-*.dll 138752 129536 7%
rustc_privacy-*.dll 202752 188416 7%
rustc_resolve-*.dll 1125888 960512 15%
rustc_save_analysis-*.dll 1490432 988160 34%
rustc_trans-*.dll 2161152 1667584 23%
rustc_typeck-*.dll 2527744 1928704 24%
rustc-*.dll 7609856 3973632 48%
rustdoc-*.dll 3517440 2946048 16%
serialize-*.dll 600064 244224 59%
std-*.dll 2287104 880640 61%
syntax_ext-*.dll 1174528 1034240 12%
syntax_pos-*.dll 178688 113152 37%
syntax-*.dll 5542400 3714048 33%
term-*.dll 286720 244224 15%
test-*.dll 408576 317440 22%
-------------------------------------------------------
Total 70657024 58680832 17%

Of these DLLs, the ones that exist in /bin correspond directly to space savings, since the import libraries won't be distributed. For those in rustlib/<target>/lib, we have to distribute the import libraries too, so there will be an overall size increase for that specific folder, due to import libraries seemingly storing the metadata in a less efficient form:

 Before After Gained
101226956 121319960 20%

Overall, the losses and gains balance out, and there is almost no change to the overall rust distribution size. However, there are real savings for programs built with rust which use dynamic linking. Even the smallest dynamically linked rust program saves ~1.4mb due to the reduced size of std-*.dll, which must be distributed with the main binary.

[alexcrichton]

Ok but to clarify, we don't know of any programs using dynamic linking? (it's barely supported by Cargo/rustc).

Also to clarify, what do you mean that a dynamically linked program will save memory? The contents of the dll don't go into the executable? Do you mean the metadata sections don't need to get loaded into memory?

[Diggsey]

I didn't mention memory? However, now that you bring it up, the DLLs being smaller means that they should use less memory when loaded.

As far as programs using dynamic linking: the RLS, clippy, and rust itself use dynamic linking, not because they rely on a plugin architecture (which AFAIK is the only benefit of dynamic linking not well supported by rust atm) but because there are multiple binaries with common dependencies, and so dynamic linking has the potential to save space.

Most programs (unlike rust) are not compilers, and so would not need to distribute the import libraries.

[alexcrichton]

Sorry I meant that you claimed a rust program save 1.4mb, but I don't know where this is coming from.

None of those programs distribute dlls though, so I don't think there's any gains to be won there?

It sounds like this change as-is does:

• Increases the size of the distribution
• May decrease the memory footprint of a process (this isn't measured one way or another)
• Adds more special casing to the backend

Overall I'm not particularly in favor of this given these numbers unfortunately.

[alexcrichton]

ping @Diggsey, any thoughts on my last comment?

[Diggsey]

None of those programs distribute dlls though, so I don't think there's any gains to be won there?

If clippy were to ship in binary form, it should be distributing those DLLs. RLS doesn't need to because it's going to be shipped as part of rust.

Overall, I think the biggest point in favour of this is libstd. People are going to be dynamically linking their rust programs as support for it improves, and libstd, which would need to be distributed in every case, sees the single biggest space saving with these changes.

[alexcrichton]

But none of the reflects reality? The one user shipping dlls, the compiler, is hurt by this change?

[alexcrichton]

none of those*

[Diggsey]

Not to my knowledge, although I wouldn't necessarily know - for example, do either servo or firefox dynamically link their rust components?

Based on comparable C/C++ applications, distributing DLLs is exceedingly common. However, if you think that rust is sufficiently far from reaching that point that it's not worth optimizing for yet, then I'll close this.

[bors]

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

[nrc]

triage: we discussed this in the tools meeting. It sounds like the benefits of this PR will not be realised until we have better dynamic library support and it adds complexity to an already complex part of the code base. Closing for now, potentially worth revisiting if/when we distributing DLLs is more common-place.

@michaelwoerister suggested that an alternative that works today is making a cdylib where we already strip metadata. This works even with Rust code, as long as you only need the C API.