Add support for -Zembed-metadata by Kobzol · Pull Request #15378 · rust-lang/cargo (original) (raw)

What does this PR try to resolve?

This PR adds Cargo integration for the new unstable -Zembed-metadata rustc flag, which was implemented in rust-lang/rust#137535 (tracking issue). The new behavior has to be enabled explicitly using a new unstable CLI flag -Zno-embed-metadata.

Tracking issue: #15495

The -Zembed-metadata=no rustc flag can reduce disk usage of compiled artifacts, and also the size of Rust dynamic library artifacts shipped to users. However, it is not enough to just pass this flag through RUSTFLAGS; it needs to be integrated within Cargo, because it interacts with how the --emit flag is passed to rustc, and also how --extern args are passed to the final linked artifact build by Cargo. Furthermore, using the flag for all crates in a crate graph compiled by Cargo would be suboptimal (this will all be described below).

When you pass -Zembed-metadata=no to rustc, it will not store Rust metadata into the compiled artifact. This is important when compiling libs/rlibs/dylibs, since it reduces their size on disk. However, this also means that everytime we use this flag, we have to make sure that we also:

• Include metadata in the --emit flag to generate a .rmeta file, otherwise no metadata would be generated whatsoever, which would mean that the artifact wouldn't be usable as a dependency.
• Pass also --extern <dep>=<path>.rmeta when compiling the final linkable artifact. Before, Cargo would only pass --extern <dep>=<path>.[rlib|so|dll]. Since with -Zembed-metadata=no, the metadata is only in the .rmeta file and not in the rlib/dylib, this is needed to help rustc find out where the metadata lies.
  • Note: this essentially doubles the cmdline length when compiling the final linked artifact. Not sure if that is a concern.

The two points above is what this PR implements, and why this rustc flag needs Cargo integration.

The -Zembed-metadata flag is only passed to libs, rlibs and dylibs. It does not seem to make sense for other crate types. The one situation where it might make sense are proc macros, but according to bjorn3 (who initially came up with the idea for -Zembed-metadata, it isn't really worth it).

Here is a table that summarizes the changes in passed flags and generated files on disk for rlibs and dylibs:

Behavior for other target kinds/crate types should be unchanged.

From the table above, we can see two benefits of using -Zembed-metadata=no:

• For rlibs/dylibs, we no longer store their metadata twice in the target directory, thus reducing target directory size.
• For dylibs, we store esssentially the same amount of data on disk, but the benefit is that the metadata is now in a separate .rmeta file. This means that you can ship the dylib (.so/.dll) to users without also shipping the metadata. This would slightly reduce e.g. the size of the shipped rustc toolchains (note that the size reduction here is after the toolchain has been already heavily compressed).

Note that if this behavior ever becomes the default, it should be possible to simplify the code quite a bit, and essentially merge the requires_upstream_objects and benefits_from_split_metadata functions.

I did a very simple initial benchmark to evaluate the space savings on cargo itself and hyperqueue (a mid-size crate from my work) using cargo build and cargo build --release with and without -Zembed-metadata=no:

For debug/incremental builds, the effect is smaller, as the artifact disk usage is dwarfed by incremental artifacts and debuginfo. But for (non-incremental) release builds, the disk savings (and also performed I/O operations) are significantly reduced.

How should we test and review this PR?

I wrote two basic tests. The second one tests a situation where a crate depends on a dylib dependency, which is quite rare, but the behavior of this has actually changed in this PR (see comparison table above). Testing this on various real-world projects (or even trying to enable it by default across the whole Cargo suite?) might be beneficial.

Unresolved questions

Is this a breaking change?

With this new behavior, dylibs and rlibs will no longer contain metadata. If they are compiled with Cargo, that shouldn't matter, but other build systems might have to adapt.

Should this become the default?

I think that in terms of disk size usage and performed I/O operations, it is a pure win. It should either generate less disk data (for rlibs) or the ~same amount of data for dylibs (the data will be a bit larger, because the dylib will still contain a metadata stub header, but that's like 50 bytes and doesn't scale with the size of the dylib, so it's negligible).

So I think that eventually, we should just do this by default in Cargo, unless some concerns are found. I suppose that before stabilizing we should also benchmark the effect on compilation performance.