-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Reduce formatting width
and precision
to 16 bits
#136932
Conversation
@bors try |
…try> Reduce formatting `width` and `precision` to 16 bits This is reduces the `width` and `precision` fields in format strings to 16 bits. They are currently full `usize`s, but it's a bit nonsensical that we need to support the case where someone wants to pad their value to eighteen quintillion spaces and/or have eighteen quintillion digits of precision. This is a breaking change, but probably affects virtually no code. Let's do a crater run to find out. Marking this as experiment for now. --- There are three ways to set a width or precision today: 1. Directly a formatting string, e.g. `println!("{a:1234}")` 2. Indirectly in a formatting string, e.g. `println!("{a:width$}", width=1234)` 3. Through the unstable `FormattingOptions::width` method. This PR: - Adds a compiler error for 1. (`println!("{a:9999999}")` no longer compiles and gives a clear error.) - Adds a runtime check for 2. (`println!("{a:width$}, width=9999999)` will panic.) - Changes the signature of `FormattingOptions::width` to take a `u16` instead. --- Additional context: All the formatting flags and options are currently: - The `+` flag (1 bit) - The `-` flag (1 bit) - The `#` flag (1 bit) - The `0` flag (1 bit) - The `x?` flag (1 bit) - The `X?` flag (1 bit) - The alignment (2 bits) - The fill character (21 bits) - Whether a width is specified (1 bit) - Whether a precision is specified (1 bit) - If used, the width (a full usize) - If used, the precision (a full usize) Everything except the last two can simply fit in a `u32` (those add up to 31 bits in total). If we can accept a max width and precision of u16::MAX, we can make a `FormattingOptions` that is exactly 64 bits in size; the same size as a thin reference on most platforms. If, additionally, we also limit the number of formatting arguments to u16::MAX, we can also reduce the size of `fmt::Arguments` (that is, of a `format_args!()` expression).
tests/mir-opt/funky_arms.float_to_exponential_common.GVN.panic-abort.diff
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
☀️ Try build successful - checks-actions |
@craterbot run mode=build-and-test |
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
7355738
to
ccb9429
Compare
As a data point, I do use write!(self.w, "\n{:1$}]", "", self.cur_indent)?;
write!(f, "{:len$}", "", len = (stride - 1) * 4)?; |
The type will stay a |
Reduce FormattingOptions to 64 bits This reduces FormattingOptions from 6-7 machine words (384 bits on 64-bit platforms, 224 bits on 32-bit platforms) to just 64 bits (a single register on 64-bit platforms). This PR includes rust-lang#136932, which reduces the width and precision options to 16 bits, to make it all fit. Before: ```rust pub struct FormattingOptions { flags: u32, // only 6 bits used fill: char, align: Option<Alignment>, width: Option<usize>, precision: Option<usize>, } ``` After: ```rust pub struct FormattingOptions { /// Bits: /// - 0: `+` flag [rt::Flag::SignPlus] /// - 1: `-` flag [rt::Flag::SignMinus] /// - 2: `#` flag [rt::Flag::Alternate] /// - 3: `0` flag [rt::Flag::SignAwareZeroPad] /// - 4: `x?` flag [rt::Flag::DebugLowerHex] /// - 5: `X?` flag [rt::Flag::DebugUpperHex] /// - 6-7: Alignment (0: Left, 1: Right, 2: Center, 3: Unknown) /// - 8: Width flag (if set, the width field below is used) /// - 9: Precision flag (if set, the precision field below is used) /// - 10: unused /// - 11-31: fill character (21 bits, a full `char`) flags: u32, /// Width if width flag above is set. Otherwise, always 0. width: u16, /// Precision if precision flag above is set. Otherwise, always 0. precision: u16, } ```
🎉 Experiment
|
Crater results: Only two regressions are caused by an error or panic from this change.
|
1fe3f1b
to
2ce0205
Compare
@bors r+ rollup=iffy (might want to bisect here) |
…=scottmcm Reduce formatting `width` and `precision` to 16 bits This is part of rust-lang#99012 This is reduces the `width` and `precision` fields in format strings to 16 bits. They are currently full `usize`s, but it's a bit nonsensical that we need to support the case where someone wants to pad their value to eighteen quintillion spaces and/or have eighteen quintillion digits of precision. By reducing these fields to 16 bit, we can reduce `FormattingOptions` to 64 bits (see rust-lang#136974) and improve the in memory representation of `format_args!()`. (See additional context below.) This also fixes a bug where the width or precision is silently truncated when cross-compiling to a target with a smaller `usize`. By reducing the width and precision fields to the minimum guaranteed size of `usize`, 16 bits, this bug is eliminated. This is a breaking change, but affects almost no existing code. --- Details of this change: There are three ways to set a width or precision today: 1. Directly a formatting string, e.g. `println!("{a:1234}")` 2. Indirectly in a formatting string, e.g. `println!("{a:width$}", width=1234)` 3. Through the unstable `FormattingOptions::width` method. This PR: - Adds a compiler error for 1. (`println!("{a:9999999}")` no longer compiles and gives a clear error.) - Adds a runtime check for 2. (`println!("{a:width$}, width=9999999)` will panic.) - Changes the signatures of the (unstable) `FormattingOptions::[get_]width` methods to use a `u16` instead. --- Additional context for improving `FormattingOptions` and `fmt::Arguments`: All the formatting flags and options are currently: - The `+` flag (1 bit) - The `-` flag (1 bit) - The `#` flag (1 bit) - The `0` flag (1 bit) - The `x?` flag (1 bit) - The `X?` flag (1 bit) - The alignment (2 bits) - The fill character (21 bits) - Whether a width is specified (1 bit) - Whether a precision is specified (1 bit) - If used, the width (a full usize) - If used, the precision (a full usize) Everything except the last two can simply fit in a `u32` (those add up to 31 bits in total). If we can accept a max width and precision of u16::MAX, we can make a `FormattingOptions` that is exactly 64 bits in size; the same size as a thin reference on most platforms. If, additionally, we also limit the number of formatting arguments, we can also reduce the size of `fmt::Arguments` (that is, of a `format_args!()` expression).
…=scottmcm Reduce formatting `width` and `precision` to 16 bits This is part of rust-lang#99012 This is reduces the `width` and `precision` fields in format strings to 16 bits. They are currently full `usize`s, but it's a bit nonsensical that we need to support the case where someone wants to pad their value to eighteen quintillion spaces and/or have eighteen quintillion digits of precision. By reducing these fields to 16 bit, we can reduce `FormattingOptions` to 64 bits (see rust-lang#136974) and improve the in memory representation of `format_args!()`. (See additional context below.) This also fixes a bug where the width or precision is silently truncated when cross-compiling to a target with a smaller `usize`. By reducing the width and precision fields to the minimum guaranteed size of `usize`, 16 bits, this bug is eliminated. This is a breaking change, but affects almost no existing code. --- Details of this change: There are three ways to set a width or precision today: 1. Directly a formatting string, e.g. `println!("{a:1234}")` 2. Indirectly in a formatting string, e.g. `println!("{a:width$}", width=1234)` 3. Through the unstable `FormattingOptions::width` method. This PR: - Adds a compiler error for 1. (`println!("{a:9999999}")` no longer compiles and gives a clear error.) - Adds a runtime check for 2. (`println!("{a:width$}, width=9999999)` will panic.) - Changes the signatures of the (unstable) `FormattingOptions::[get_]width` methods to use a `u16` instead. --- Additional context for improving `FormattingOptions` and `fmt::Arguments`: All the formatting flags and options are currently: - The `+` flag (1 bit) - The `-` flag (1 bit) - The `#` flag (1 bit) - The `0` flag (1 bit) - The `x?` flag (1 bit) - The `X?` flag (1 bit) - The alignment (2 bits) - The fill character (21 bits) - Whether a width is specified (1 bit) - Whether a precision is specified (1 bit) - If used, the width (a full usize) - If used, the precision (a full usize) Everything except the last two can simply fit in a `u32` (those add up to 31 bits in total). If we can accept a max width and precision of u16::MAX, we can make a `FormattingOptions` that is exactly 64 bits in size; the same size as a thin reference on most platforms. If, additionally, we also limit the number of formatting arguments, we can also reduce the size of `fmt::Arguments` (that is, of a `format_args!()` expression).
…=scottmcm Reduce formatting `width` and `precision` to 16 bits This is part of rust-lang#99012 This is reduces the `width` and `precision` fields in format strings to 16 bits. They are currently full `usize`s, but it's a bit nonsensical that we need to support the case where someone wants to pad their value to eighteen quintillion spaces and/or have eighteen quintillion digits of precision. By reducing these fields to 16 bit, we can reduce `FormattingOptions` to 64 bits (see rust-lang#136974) and improve the in memory representation of `format_args!()`. (See additional context below.) This also fixes a bug where the width or precision is silently truncated when cross-compiling to a target with a smaller `usize`. By reducing the width and precision fields to the minimum guaranteed size of `usize`, 16 bits, this bug is eliminated. This is a breaking change, but affects almost no existing code. --- Details of this change: There are three ways to set a width or precision today: 1. Directly a formatting string, e.g. `println!("{a:1234}")` 2. Indirectly in a formatting string, e.g. `println!("{a:width$}", width=1234)` 3. Through the unstable `FormattingOptions::width` method. This PR: - Adds a compiler error for 1. (`println!("{a:9999999}")` no longer compiles and gives a clear error.) - Adds a runtime check for 2. (`println!("{a:width$}, width=9999999)` will panic.) - Changes the signatures of the (unstable) `FormattingOptions::[get_]width` methods to use a `u16` instead. --- Additional context for improving `FormattingOptions` and `fmt::Arguments`: All the formatting flags and options are currently: - The `+` flag (1 bit) - The `-` flag (1 bit) - The `#` flag (1 bit) - The `0` flag (1 bit) - The `x?` flag (1 bit) - The `X?` flag (1 bit) - The alignment (2 bits) - The fill character (21 bits) - Whether a width is specified (1 bit) - Whether a precision is specified (1 bit) - If used, the width (a full usize) - If used, the precision (a full usize) Everything except the last two can simply fit in a `u32` (those add up to 31 bits in total). If we can accept a max width and precision of u16::MAX, we can make a `FormattingOptions` that is exactly 64 bits in size; the same size as a thin reference on most platforms. If, additionally, we also limit the number of formatting arguments, we can also reduce the size of `fmt::Arguments` (that is, of a `format_args!()` expression).
Rollup of 16 pull requests Successful merges: - rust-lang#126856 (remove deprecated tool `rls`) - rust-lang#136932 (Reduce formatting `width` and `precision` to 16 bits) - rust-lang#137314 (change definitely unproductive cycles to error) - rust-lang#137612 (Update bootstrap to edition 2024) - rust-lang#138002 (Disable CFI for weakly linked syscalls) - rust-lang#138052 (strip `-Wlinker-messages` wrappers from `rust-lld` rmake test) - rust-lang#138063 (Improve `-Zunpretty=hir` for parsed attrs) - rust-lang#138109 (make precise capturing args in rustdoc Json typed) - rust-lang#138147 (Add maintainers for powerpc64le-unknown-linux-gnu) - rust-lang#138245 (stabilize `ci_rustc_if_unchanged_logic` test for local environments) - rust-lang#138296 (Remove `AdtFlags::IS_ANONYMOUS` and `Copy`/`Clone` condition for anonymous ADT) - rust-lang#138300 (add tracking issue for unqualified_local_imports) - rust-lang#138307 (Allow specifying glob patterns for try jobs) - rust-lang#138313 (Update books) - rust-lang#138315 (use next_back() instead of last() on DoubleEndedIterator) - rust-lang#138318 (Rustdoc: remove a bunch of `@ts-expect-error` from main.js) r? `@ghost` `@rustbot` modify labels: rollup
@jieyouxu |
If this is perf-sensitive, then it should be marked as rollup=never, or it can be perf-build specifically post rollup. The size of the rollup has no bearing to this, you would have to perf build separately in a 5 PR rollup anyway. |
For future bisection, |
☀️ Test successful - checks-actions |
A job failed! Check out the build log: (web) (plain) Click to see the possible cause of the failure (guessed by this bot)
|
Finished benchmarking commit (374ce1f): comparison URL. Overall result: ❌ regressions - please read the text belowOur benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary -1.1%, secondary 2.8%)This 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.
CyclesResults (secondary -1.5%)This 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 sizeResults (primary -0.0%, secondary 0.0%)This 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: 782.162s -> 782.387s (0.03%) |
This is part of #99012
This is reduces the
width
andprecision
fields in format strings to 16 bits. They are currently fullusize
s, but it's a bit nonsensical that we need to support the case where someone wants to pad their value to eighteen quintillion spaces and/or have eighteen quintillion digits of precision.By reducing these fields to 16 bit, we can reduce
FormattingOptions
to 64 bits (see #136974) and improve the in memory representation offormat_args!()
. (See additional context below.)This also fixes a bug where the width or precision is silently truncated when cross-compiling to a target with a smaller
usize
. By reducing the width and precision fields to the minimum guaranteed size ofusize
, 16 bits, this bug is eliminated.This is a breaking change, but affects almost no existing code.
Details of this change:
There are three ways to set a width or precision today:
println!("{a:1234}")
println!("{a:width$}", width=1234)
FormattingOptions::width
method.This PR:
println!("{a:9999999}")
no longer compiles and gives a clear error.)println!("{a:width$}, width=9999999)
will panic.)FormattingOptions::[get_]width
methods to use au16
instead.Additional context for improving
FormattingOptions
andfmt::Arguments
:All the formatting flags and options are currently:
+
flag (1 bit)-
flag (1 bit)#
flag (1 bit)0
flag (1 bit)x?
flag (1 bit)X?
flag (1 bit)Everything except the last two can simply fit in a
u32
(those add up to 31 bits in total).If we can accept a max width and precision of u16::MAX, we can make a
FormattingOptions
that is exactly 64 bits in size; the same size as a thin reference on most platforms.If, additionally, we also limit the number of formatting arguments, we can also reduce the size of
fmt::Arguments
(that is, of aformat_args!()
expression).