-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
MIR-only rlibs #119017
base: master
Are you sure you want to change the base?
MIR-only rlibs #119017
Conversation
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
[perf experiment] (crudely) implement MIR-only rlibs I realized that `-Zcross-crate-inline-threshold=always` is now basically MIR-only rlibs. So if nothing else, this is an easy way to study the perf implications of such a design. Pondering this because it would be neat to codegen MIR for the standard library differently based on build flags passed by an end user. r? `@ghost`
This comment has been minimized.
This comment has been minimized.
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (f4568e9): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Bootstrap: 672.461s -> 1381.575s (105.45%) |
741a7a7
to
9fc5b9f
Compare
Failed to set assignee to
|
☔ The latest upstream changes (presumably #119843) made this pull request unmergeable. Please resolve the merge conflicts. |
The MCP has been accepted |
👍 This PR is still not really mergeable because it breaks proc macros. I think because I'm monomorphizing the allocator shims wrong. |
52e910a
to
623b626
Compare
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #121644) made this pull request unmergeable. Please resolve the merge conflicts. |
623b626
to
61bdbd7
Compare
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #122037) made this pull request unmergeable. Please resolve the merge conflicts. |
61bdbd7
to
46dd526
Compare
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #122568) made this pull request unmergeable. Please resolve the merge conflicts. |
46dd526
to
4b0f58c
Compare
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #124558) made this pull request unmergeable. Please resolve the merge conflicts. |
How feasible would it be to opt into a MIR-only rlib for a particular dependency? That might sidestep some of the difficulties associated with using it everywhere, while allowing large crates with very few items used to opt into it to reduce how much gets compiled. |
If you want to try using this to cut down build times, you can investigate whether it would help by setting But thanks for pinging this PR! I'll try to rebase it up soon just to keep it alive. |
I tested this on one of my projects: Baseline:
With the three biggest crates in the dependency tree (which I use a tiny fraction of the items of) using
|
4b0f58c
to
c776c87
Compare
Note that CI may pass, because in line with:
I removed the line that was making the compiler bootstrap with |
This comment has been minimized.
This comment has been minimized.
c776c87
to
2986d31
Compare
This comment has been minimized.
This comment has been minimized.
Update (same project, flags applied to the same set of 3 large crates with most items unused):
|
☔ The latest upstream changes (presumably #132173) made this pull request unmergeable. Please resolve the merge conflicts. |
2986d31
to
ae0521d
Compare
Status update: The strategy I implemented in this PR is to make builds of non-rlib crates monomorphize everything they need. I was quite thorough about this and had to add a handful of hacks to make it happen.
But I now believe that was a mistake, because of how it breaks proc-macro crates. I think those crates can only work if they are able to cooperate with the compiler correctly to share types across the proc-macro bridge. Currently this is achieved by linking both against the libstd dylib, which contains monomorphizations of the allocator shims and probably a handful of other things. So in order to make this work, I at least need to figure out how to make the compiler expose all the monomorphizations of the standard library, and make the proc-macro crates link against those instead of monomorphizing their own.
Originally I was just trying to imitate MIR-only rlibs with
-Zcross-crate-inline-threshold=yes
, but I think I have enough skill to actually implement the behavior. We shall see.At time of writing, set
RUSTFLAGS=-Zmir-only-libs
and if each session that Cargo kicks off only compiles an rlib or an executable, every rlib should contain only MIR and the build should work. Any other configuration probably explodes unceremoniously.r? @ghost