Skip to content
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

Add -C hint-mostly-unused to tell rustc that most of a crate will go unused #135656

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

joshtriplett
Copy link
Member

@joshtriplett joshtriplett commented Jan 17, 2025

This hint allows the compiler to optimize its operation based on this assumption, in order to compile faster. This is a hint, and does not guarantee any particular behavior.

This option can substantially speed up compilation if applied to a large dependency where the majority of the dependency does not get used. This flag may slow down compilation in other cases.

Currently, this option makes the compiler defer as much code generation as possible from functions in the crate, until later crates invoke those functions. Functions that never get invoked will never have code generated for them. For instance, if a crate provides thousands of functions, but only a few of them will get called, this flag will result in the compiler only doing code generation for the called functions. (This uses the same mechanisms as cross-crate inlining of functions.) This does not affect extern functions, or functions marked as #[inline(never)].

This option has already existed in nightly as -Zcross-crate-inline-threshold=always for some time, and has gotten testing in that form. However, this option is still unstable, to give an opportunity for wider testing in this form.

Some performance numbers, based on a crate with many dependencies having just one large dependency set to -C hint-mostly-unused (using Cargo's profile-rustflags option):

A release build went from 4m32s to 2m06s.

A non-release build went from 2m13s to 1m24s.

@joshtriplett joshtriplett added I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 17, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 17, 2025

r? @GuillaumeGomez

rustbot has assigned @GuillaumeGomez.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot
Copy link
Collaborator

rustbot commented Jan 17, 2025

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 17, 2025
@GuillaumeGomez
Copy link
Member

r? compiler

@rustbot rustbot assigned fee1-dead and unassigned GuillaumeGomez Jan 17, 2025
@joshtriplett joshtriplett added the relnotes Marks issues that should be documented in the release notes of the next release. label Jan 17, 2025
@workingjubilee
Copy link
Member

This should be added as an unstable option first?

@joshtriplett
Copy link
Member Author

@workingjubilee This option has already existed in nightly as -Zcross-crate-inline-threshold=always for some time, and has gotten testing in that form.

The only difference being made between that option and this one is that this one respects #[inline(never)].

@workingjubilee workingjubilee added needs-fcp This change is insta-stable, so needs a completed FCP to proceed. I-compiler-nominated Nominated for discussion during a compiler team meeting. labels Jan 17, 2025
@rust-log-analyzer

This comment was marked as resolved.

@workingjubilee

This comment was marked as resolved.

@workingjubilee
Copy link
Member

@joshtriplett:

@workingjubilee This option has already existed in nightly as -Zcross-crate-inline-threshold=always for some time, and has gotten testing in that form.

The only difference being made between that option and this one is that this one respects #[inline(never)].

I see. I suppose that makes sense, though it brings these thoughts to mind:

  • In general, T-compiler has a much lower threshold for unstable flags that are not intended for serious or permanent usage downstream. They're often added as debugging aids. I am not sure if this was intended for public usage when it was originally added.
  • All inlining annotations are hints, and this is also noted as a hint, yet it changes the behavior of another inlining hint, and in that way deviates from its unstable variation.
  • Notably, inline(never) is often relied on to some degree by programmers as working.
  • This describes itself as a hint, but also is motivated on the basis that it will be very impactful, which suggests its behavior will be immediately relied on. The combination of relying-on-inlining-but-also-relying-on-not-inlining makes me wonder if it is definitely correct? There are a lot of weird nuances to our inlining rules.

As a result, I am curious if the rest of T-compiler believes changing the behavior with respect to inline(never) is trivial.

Perhaps this should land with a test that demonstrates that it respects #[inline(never)] correctly, now?

@workingjubilee
Copy link
Member

( I'm also kinda curious if we should adjust how we handle cross-crate inlining by default, tbh. )

@lqd
Copy link
Member

lqd commented Jan 18, 2025

You may have missed it Jubilee, so hop on to https://rust-lang.zulipchat.com/#narrow/stream/233931-xxx/topic/Add.20.60-C.20hint-mostly-unused.60.20option.20compiler-team.23829 to ask your cool questions and comments :)

…o unused

This hint allows the compiler to optimize its operation based on this
assumption, in order to compile faster. This is a hint, and does not
guarantee any particular behavior.

This option can substantially speed up compilation if applied to a large
dependency where the majority of the dependency does not get used. This flag
may slow down compilation in other cases.

Currently, this option makes the compiler defer as much code generation as
possible from functions in the crate, until later crates invoke those
functions. Functions that never get invoked will never have code generated for
them. For instance, if a crate provides thousands of functions, but only a few
of them will get called, this flag will result in the compiler only doing code
generation for the called functions. (This uses the same mechanisms as
cross-crate inlining of functions.) This does not affect `extern` functions, or
functions marked as `#[inline(never)]`.

Some performance numbers, based on a crate with many dependencies having
just *one* large dependency set to `-C hint-mostly-unused` (using
Cargo's `profile-rustflags` option):

A release build went from 4m32s to 2m06s.

A non-release build went from 2m13s to 1m24s.
@joshtriplett joshtriplett removed relnotes Marks issues that should be documented in the release notes of the next release. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. labels Jan 18, 2025
@@ -172,6 +172,25 @@ values:

The default if not specified depends on the target.

## hint-mostly-unused
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the flag is now unstable/nightly-gated, this documentation should be moved to the unstable-book (under src/doc/unstable-book/src/compiler-flags) and a tracking issue should be created.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for a tracking issue before consensus tho right ? This still needs fcp anyways. (And given the reservations in the thread some more tests and all will be needed before stabilization.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, the creation of the tracking issue can wait until the MCP is accepted and the PR nearing approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-compiler-nominated Nominated for discussion during a compiler team meeting. I-compiletime Issue: Problems and improvements with respect to compile times. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants