Use rust-lld on windows rustdoc in config_fast_builds.toml

[SkiFire13]

Objective

• Rustdoc doesn't seem to follow cargo's linker setting
• Improves the situation in bevy_gizmos doc tests hang compute when run locally #12207

Solution

• Explicitly set the linker in rustdoc flags

Testing

• I tested this change on Windows and it significantly improves testing performances (can't give an exact estimate since they got stuck before this change)

---

Note: I avoided changing the settings on Linux and MacOS because I can't test on those platforms. It would be nice if someone could test similar changes there and report so they can be done on all major platforms.

[BD103]

I'm confused, doesn't -C just mean a generic codegen option and not the linker specifically? Shouldn't this be something like -Clink-args=-fuse-ld=rust-lld.exe?

[SkiFire13]

Oops I think I messed up when copying from my local config.toml (which is probably why I didn't notice it being wrong). It was supposed to be -Clinker=rust-lld.exe, but I'm not sure what the difference with -Clink-args=-fuse-ld=rust-lld.exe would be.

[BD103]

-Clink-args=-fuse-ld=rust-lld.exe is likely a Clang / GCC-only command, so I would just focus on finding the Windows MSVS version.

[SkiFire13]

I've fixed the typo to -Clinker=rust-lld.exe. -Clink-args=-fuse-ld=rust-lld.exe doesn't appear to work, maybe -fuse-ld is an argument specific to GCC and Clang's linker? Here it would be passed to MSVC's linker which I guess doesn't understand it.

[BD103]

Note: I avoided changing the settings on Linux and MacOS because I can't test on those platforms. It would be nice if someone could test similar changes there and report so they can be done on all major platforms.

I tested using LLD on a MacOS M1 chip, and it was slower. This is due to Apple's default ld64 being faster. I still think it should be kept for Windows, though, due to the performance gains.

If possible, could someone test on x86 MacOS and x86 Linux? I'd used:

[target.x86_64-apple-darwin]
rustflags = [
  # ...
]
rustdocflags = ["-Clink-arg=-fuse-ld=/usr/local/opt/llvm/bin/ld64.lld"]

[target.x86_64-unknown-linux-gnu]
linker = "clang"
rustflags = [
  # ...
]
rustdocflags = ["-Clink-arg=-fuse-ld=lld"] # Also try testing the Mold linker, too!

[BD103]

I'm good with these changes, as long as we can test this on Linux and x86 MacOS too. :)

[mockersf]

This file is here for Bevy users to set up their own projects, not really for Bevy contributors. Nowhere are they pointed to this file. This should go into the contributor guide rather than here.

I doubt we will have a lot of tests available for x86 macs, I'm ok with not mentioning them

[SkiFire13]

This file is here for Bevy users to set up their own projects, not really for Bevy contributors.

Why does it have an alias for cargo run -p ci then?

That said, would it make sense to split it into a config for contributors and one for users?

[mockersf]

This file is here for Bevy users to set up their own projects, not really for Bevy contributors.

Why does it have an alias for cargo run -p ci then?

Because I didn't saw the PR that added that and don't think it should have been merged. This file doesn't have the default name specifically to not be used by Bevy contributors automatically

[BD103]

Why does it have an alias for cargo run -p ci then?

I added that feature because I thought config_fast_builds.toml was targeted towards contributors. Honestly I still feel this way, and that a user-oriented version should be in the documentation instead.

[janhohenheim]

Even for end users this is a good default imo. At least plugin authors on Windows will be happy about this.
If we find speed improvements for other platforms, we can always add them in follow-up PRs.

[alice-i-cecile]

@mockersf I agree with @janhohenheim: this is useful for Bevy users directly. Faster doc tests are valuable for library authors, especially since this is a super subtle footgun.

[janhohenheim]

I've tested this on Windows and I get the following when compiling with dynamic_linking on 0.14.0-rc.4:

Test executable failed (exit code: 0xc0000135).

My doctest is as simple as this:

/// ```
/// use bevy::prelude::*;
/// ```
struct Foo;

This error code means "dynamic library not found". I then ran my test again without dynamic_linking and it runs. As such, introducing this config for Windows is a footgun.

Note that this issue does not pop up when using LLD or mold as a rustdoc linker on Linux and seems to be Windows-specific.

I'd be glad if someone else could check this on their machine as well. If this is also the case for others, I'd vote to only add this flag to non-Windows targets, which probably means closing this PR.

Edit: nvm, I get the same behavior whether or not I have the rustdocflags on or not. My issue seems to not be related. Still, I'd be happy if someone else ran a doctest with the following config on Windows:

[target.x86_64-pc-windows-msvc]
linker = "rust-lld.exe"
rustdocflags = ["-Clinker=rust-lld.exe"]
rustflags = [
    "-Zshare-generics=n",
    "-Zthreads=0",
]

[BD103]

Marking this as "Needs Testing:" someone with an x86-64 Windows machine should test the configuration that @janhohenheim used.

[SkiFire13]

@janhohenheim which nightly version are you using (since you specified nightly-only flags)?

Also, are you testing that doctest in isolation (i.e. in its own crate and workspace, without dependencies) or are you testing it from inside Bevy? It would be nice if this was reproducible with a small crate so the root cause can be more easily identified.

[janhohenheim]

@SkiFire13 nightly 0.18.0
Setting up a minimal crate right now.

@BD103 turns out my doctest does not run with dynamic linking even when my .cargo/config.toml is entirely empty, so this definitely has nothing to do with this PR. I'll open a new issue once I have the minimal example ready. Feel free to remove the "Needs Testing" label and merge, if you want.

[SkiFire13]

@SkiFire13 nightly 0.18.0

I'm not sure what you're talking about... Could you post the output of rustc +nightly --version?

@BD103 turns out my doctest does not run with dynamic linking even when my .cargo/config.toml is entirely empty, so this definitely has nothing to do with this PR. I'll open a new issue once I have the minimal example ready.

On windows if you enable the dynamic_linking feature you also need to enable optimizations (which are disabled by default for tests)

[janhohenheim]

@SkiFire13 sorry, I was typing Bevy 0.14.0 so many times over the last days that I automatically started my version by typing 0.1, haha. I mean 1.80.0 :)

Wait, so this snippet here is ignored by tests?

# Enable a small amount of optimization in debug mode
[profile.dev]
opt-level = 1

# Enable high optimizations for dependencies (incl. Bevy), but not for our code:
[profile.dev.package."*"]
opt-level = 3

[SkiFire13]

@SkiFire13 sorry, I was typing Bevy 0.14.0 so many times over the last days that I automatically started my version by typing 0.1, haha. I mean 1.80.0 :)

That's kinda old, so I guess this is not some new rustc bug.

Wait, so this snippet here is ignored by tests?

That applies to the dev profile, which is the one used by cargo build/cargo run when not specifying --release. cargo test uses the test profile instead, so you need to also do this:

[profile.test]
opt-level = 1

[profile.test.package."*"]
opt-level = 3

[janhohenheim]

@SkiFire13 that sounds very much like my issue then, thanks. I'll recompile everything with those settings and see if that fixes it. It will take a moment since I've disabled LLD, sccache, etc. for this.

[janhohenheim]

@SkiFire13 Here's a minimal repo of the issue. Could you confirm whether running cargo test --doc on it works on Windows? I tried adding the [profile.test] parts, but they did not help. In any case, I noticed that if the optimizations were the problem, I would probably have gotten a build-time error.

[SkiFire13]

@SkiFire13 Here's a minimal repo of the issue. Could you confirm whether running cargo test --doc on it works on Windows? I tried adding the [profile.test] parts, but they did not help. In any case, I noticed that if the optimizations were the problem, I would probably have gotten a build-time error.

@janhohenheim I can confirm that this results in the Test executable failed (exit code: 0xc0000135) error for me as well (tested with an empty .cargo/config.toml and no RUSTFLAGS or RUSTDOCFLAGS set).

I've also managed to further minimize the issue to remove the dependency on Bevy. You only need a crate depending on a dylib crate, and a doctest that uses any item from the dylib crate. https://github.com/SkiFire13/minimal_dylib_doctest_fail

I've also found this rust issue which seems to fit the problem rust-lang/rust#39487. It has been closed in favour of rust-lang/rustup#893, where it's mentioned that the issue is with the stdilb dll not being added to the path when running the test. Indeed if I either add %RUSTUP_HOME%\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\ to the PATH or copy the std-****.dll file in that directory to the directory where I'm running cargo test --doc then it works. However I wonder why this issue doesn't appear on Linux (I've tested on WSL and the same minimalized reproduction works fine without the workaround).

[janhohenheim]

@SkiFire13 thanks for the investigation! I'll open a new issue later. We should probably add this info somewhere so that new users don't fall into this trap.

Edit: opened up #14129