-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
std: A few code size optimizations #12616
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
// LLVM doesn't tend to inline this, presumably because begin_unwind_fmt | ||
// is #[cold] and #[inline(never)] and because this is flagged as cold | ||
// as returning !. We really do want this to be inlined, however, | ||
// because it's just a tiny wrapper. Small wins (~7K) were seen when |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7K out of? (These numbers are more useful if they have percentages with them too)
Nice wins! r=me addressing my comments, seems the travis failures might be related? |
This is a ubiquitous type in concurrent code, and the assertions are causing significant code bloat for simple operations such as reading the pointer (injecting a failure point, etc). I am testing executable sizes with no I/O implementations (everything stubbed out to return nothing), and this took the size of a libnative executable from 328K to 207K (37% reduction in size), so I think that this is one assertion that's well worth configuring off for now.
Most of these are unnecessary because we're only looking at static strings. This also moves to Vec in a few places instead of ~[T]. This didn't end up getting much of a code size win (update_log_settings is the third largest function in the executables I'm looking at), but this seems like a generally nice improvement regardless.
This removes all usage of Poly in format strings from libstd. This doesn't prevent more future strings from coming in, but it at least removes the ones for now.
This function is a tiny wrapper that LLVM doesn't want to inline, and it ends up causing more bloat than necessary. The bloat is pretty small, but it's a win of at least 7k for small executables, and I imagine that the number goes up as there are more calls to fail!().
bors
added a commit
that referenced
this pull request
Feb 28, 2014
I've been playing around with code size when linking to libstd recently, and these were some findings I found that really helped code size. I started out by eliminating all I/O implementations from libnative and instead just return an unimplemented error. In doing so, a `fn main() {}` executable was ~378K before this patch, and about 170K after the patch. These size wins are all pretty minor, but they all seemed pretty reasonable to me. With native I/O not stubbed out, this takes the size of an LTO executable from 675K to 400K.
Merged
Minor or not, getting |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jul 25, 2022
Correct target_feature completion I changed the `target_feature` to match the description given in rust-lang#12616.
flip1995
pushed a commit
to flip1995/rust
that referenced
this pull request
Apr 4, 2024
avoid an ICE in `ptr_as_ptr` when getting the def_id of a local Fixes rust-lang#12616 `Res::def_id` can panic, so avoid calling it in favor of `opt_def_id`, so we can gracefully handle resolutions that don't have a `DefId` (e.g. local variables) and get a false negative in the worst case, rather than an ICE changelog: Fix ICE in [`ptr_as_ptr`] when the cast expression is a function call to a local variable
flip1995
pushed a commit
to flip1995/rust
that referenced
this pull request
Apr 25, 2024
avoid an ICE in `ptr_as_ptr` when getting the def_id of a local Fixes rust-lang#12616 `Res::def_id` can panic, so avoid calling it in favor of `opt_def_id`, so we can gracefully handle resolutions that don't have a `DefId` (e.g. local variables) and get a false negative in the worst case, rather than an ICE changelog: Fix ICE in [`ptr_as_ptr`] when the cast expression is a function call to a local variable
flip1995
pushed a commit
to flip1995/rust
that referenced
this pull request
Apr 25, 2024
avoid an ICE in `ptr_as_ptr` when getting the def_id of a local Fixes rust-lang#12616 `Res::def_id` can panic, so avoid calling it in favor of `opt_def_id`, so we can gracefully handle resolutions that don't have a `DefId` (e.g. local variables) and get a false negative in the worst case, rather than an ICE changelog: Fix ICE in [`ptr_as_ptr`] when the cast expression is a function call to a local variable
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've been playing around with code size when linking to libstd recently, and these were some findings I found that really helped code size. I started out by eliminating all I/O implementations from libnative and instead just return an unimplemented error.
In doing so, a
fn main() {}
executable was ~378K before this patch, and about 170K after the patch. These size wins are all pretty minor, but they all seemed pretty reasonable to me. With native I/O not stubbed out, this takes the size of an LTO executable from 675K to 400K.