-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Reduce code size overhead from core::panicking::panic #22948
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
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see CONTRIBUTING.md for more information. |
LGTM, but I think this test case doesn't capture anything new, so please remove it. @alexcrichton are you ok adding an unstable method to the ArgumentsV1 trait here? |
Couldn't this call Either way, can you add documentation to |
I had added this short comment inside // To reduce formatting code size overhead, create an Arguments structure
// that does not reference a formatting function. I'll remove the test. |
Yeah that's fine, thanks! |
Let me know if I should squash and/or rebase these changes. |
Ah yes, a squash would be nice, other than that r=me, this looks great, thanks! |
Display::fmt for str calls into Formatter::pad, which is modest in size and also pulls in string-related functions for its truncation and padding abilities. For size-critical programs (e.g. embedded), this call site may be the only reason Formatter::pad is linked into the output.
a4e4c74
to
9c0057d
Compare
@alexcrichton I squashed the commits, so this PR should be ready now. |
Reduce code size overhead from core::panicking::panic core::panicking::panic currently creates an Arguments structure using format_args!("{}", expr), which formats the expr str using the Display::fmt. Display::fmt pulls in Formatter::pad, which then also pulls in string-related code for truncation and padding. If core::panicking::panic instead creates an Arguments structure with a string piece, it is possible that the Display::fmt function for str can be optimized out of the program. In my testing with a 32-bit x86 bare metal program, the change tended to save between ~100 bytes and ~5500 bytes, depending on what other panic* functions the program invokes and whether the panic_fmt lang item uses the Arguments value.
Reduce code size overhead from core::panicking::panic
core::panicking::panic currently creates an Arguments structure using
format_args!("{}", expr), which formats the expr str using the Display::fmt.
Display::fmt pulls in Formatter::pad, which then also pulls in string-related
code for truncation and padding.
If core::panicking::panic instead creates an Arguments structure with a string
piece, it is possible that the Display::fmt function for str can be optimized
out of the program.
In my testing with a 32-bit x86 bare metal program, the change tended to save
between ~100 bytes and ~5500 bytes, depending on what other panic* functions
the program invokes and whether the panic_fmt lang item uses the Arguments
value.