fmt of non-decimal radix untangled

[pascaldekloe]

Have the implementation match its decimal counterpart.

• Digit table instead of conversion functions
• Correct buffer size per radix
• Elimination of dead code for negative
• No trait abstraction for integers

Original Performance

    fmt::write_10ints_bin                                                393.03ns/iter      +/- 1.41
    fmt::write_10ints_hex                                                316.84ns/iter      +/- 1.49
    fmt::write_10ints_oct                                                327.16ns/iter      +/- 0.46

Patched Performance

    fmt::write_10ints_bin                                                392.31ns/iter      +/- 3.05
    fmt::write_10ints_hex                                                302.41ns/iter      +/- 5.48
    fmt::write_10ints_oct                                                322.01ns/iter      +/- 3.82

r? tgross35

[pascaldekloe]

https://rust.godbolt.org/z/K1hWxa7dP

[bors]

☔ The latest upstream changes (presumably #144044) made this pull request unmergeable. Please resolve the merge conflicts.

[pascaldekloe]

Can you either have a look or reassign @tgross35? Got a followup pending on this one...

[tgross35]

I'll reassign for now, but if nobody beats me to it I'll take a look on Monday

r? libs

(leaving myself assigned so it stays on my list)

[tgross35]

By the way; if you are interested, we could use some int (and float) formatting and parsing benchmarks at https://github.com/rust-lang/rustc-perf/blob/2120e3b7b8996e96858b88edefea371679a3d415/collector/runtime-benchmarks/fmt/src/main.rs (I just learned that is possible), which would mean they get run as part of our pretty extensive perf infra rather than you needing to post the results of local runs.

[pascaldekloe]

Interesting indeed @tgross35. The specialized benchmarks we have at the moment (such as write_u8_min and write_u64_max) won't scale. Think about how many benchmarks we need for fmt::LowerExp with edge cases on alignment and precision. I am curious to hear what you think of the _10ints_ concept in this patch. The idea is to have a balanced mix of common cases.

[tgross35]

Few notes, haven't looked at patches 3 & 4 yet

[tgross35]

Guess it's still on me, but this looks pretty good! Just a few small things

[rust-bors]

☀️ Try build successful (CI)
Build commit: 4984ea3 (4984ea3a0b1f82943c53b31bdd2cd29e5f0f2cf5, parent: 6d091b2baa33698682453c7bb72809554204e434)

[rust-timer]

Finished benchmarking commit (4984ea3): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.1%	[0.1%, 0.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.2%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -1.4%, secondary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.9%	[0.8%, 3.0%]	2
Regressions ❌ (secondary)	0.8%	[0.8%, 0.8%]	3
Improvements ✅ (primary)	-3.1%	[-8.4%, -1.1%]	4
Improvements ✅ (secondary)	-2.7%	[-2.7%, -2.7%]	1
All ❌✅ (primary)	-1.4%	[-8.4%, 3.0%]	6

Cycles

Results (secondary 4.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	4.6%	[3.2%, 6.6%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

Results (primary -0.1%, secondary -0.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.1%, 0.1%]	4
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.2%	[-0.2%, -0.2%]	4
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.1%]	41
All ❌✅ (primary)	-0.1%	[-0.2%, 0.1%]	8

Bootstrap: 469.09s -> 467.235s (-0.40%)
Artifact size: 376.98 MiB -> 376.94 MiB (-0.01%)

[tgross35]

Thanks for cleaning up even more formatting!

@bors r+

[bors]

📌 Commit 0291e78 has been approved by tgross35

It is now in the queue for this repository.

[bors]

⌛ Testing commit 0291e78 with merge ac7239a...

[bors]

💔 Test failed - checks-actions

[jieyouxu]

Tree sync, not approved.
@bors r-

[tgross35]

@bors2 try jobs=x86_64-gnu-llvm-19-3

r=me with that passing

[rust-bors]

💔 Test for 59e4e19 failed: CI. Failed jobs:

• try - x86_64-gnu-llvm-19-3 (web logs, extended logs)

[rust-log-analyzer]

The job x86_64-gnu-llvm-19-3 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

   Compiling std v0.0.0 (/checkout/library/std)
error[E0614]: type `u8` cannot be dereferenced
   --> library/core/src/fmt/num.rs:331:64
    |
121 | / macro_rules! impl_Display {
122 | |     ($($Signed:ident, $Unsigned:ident),* ; as $T:ident into $fmt_fn:ident) => {
123 | |
124 | |         $(
...   |
331 | |                     offset = ${concat($fmt_fn, _in_buf_small)}(*self as $T, &mut buf.buf);
    | |                                                                ^^^^^ can't be dereferenced
...   |
372 | |     };
373 | | }
    | |_- in this expansion of `impl_Display!`
...
562 |       impl_Display!(i8, u8, i16, u16, i32, u32, i64, u64, isize, usize; as u64 into display_u64);
    |       ------------------------------------------------------------------------------------------ in this macro invocation

error[E0614]: type `u16` cannot be dereferenced
   --> library/core/src/fmt/num.rs:331:64
    |
121 | / macro_rules! impl_Display {
122 | |     ($($Signed:ident, $Unsigned:ident),* ; as $T:ident into $fmt_fn:ident) => {
123 | |
124 | |         $(
...   |
331 | |                     offset = ${concat($fmt_fn, _in_buf_small)}(*self as $T, &mut buf.buf);
    | |                                                                ^^^^^ can't be dereferenced
...   |
372 | |     };
373 | | }
    | |_- in this expansion of `impl_Display!`
...
562 |       impl_Display!(i8, u8, i16, u16, i32, u32, i64, u64, isize, usize; as u64 into display_u64);
    |       ------------------------------------------------------------------------------------------ in this macro invocation

error[E0614]: type `u32` cannot be dereferenced
   --> library/core/src/fmt/num.rs:331:64
    |
121 | / macro_rules! impl_Display {
122 | |     ($($Signed:ident, $Unsigned:ident),* ; as $T:ident into $fmt_fn:ident) => {
123 | |
124 | |         $(
...   |
331 | |                     offset = ${concat($fmt_fn, _in_buf_small)}(*self as $T, &mut buf.buf);
    | |                                                                ^^^^^ can't be dereferenced
...   |
372 | |     };
373 | | }
    | |_- in this expansion of `impl_Display!`
...
562 |       impl_Display!(i8, u8, i16, u16, i32, u32, i64, u64, isize, usize; as u64 into display_u64);
    |       ------------------------------------------------------------------------------------------ in this macro invocation

error[E0614]: type `u64` cannot be dereferenced
   --> library/core/src/fmt/num.rs:331:64
    |
121 | / macro_rules! impl_Display {
122 | |     ($($Signed:ident, $Unsigned:ident),* ; as $T:ident into $fmt_fn:ident) => {
123 | |
124 | |         $(
...   |
331 | |                     offset = ${concat($fmt_fn, _in_buf_small)}(*self as $T, &mut buf.buf);
    | |                                                                ^^^^^ can't be dereferenced
...   |
372 | |     };
373 | | }
    | |_- in this expansion of `impl_Display!`
...
562 |       impl_Display!(i8, u8, i16, u16, i32, u32, i64, u64, isize, usize; as u64 into display_u64);
    |       ------------------------------------------------------------------------------------------ in this macro invocation

error[E0614]: type `usize` cannot be dereferenced
   --> library/core/src/fmt/num.rs:331:64
    |
121 | / macro_rules! impl_Display {
122 | |     ($($Signed:ident, $Unsigned:ident),* ; as $T:ident into $fmt_fn:ident) => {
123 | |
124 | |         $(
...   |
331 | |                     offset = ${concat($fmt_fn, _in_buf_small)}(*self as $T, &mut buf.buf);
    | |                                                                ^^^^^ can't be dereferenced
...   |
372 | |     };
373 | | }
    | |_- in this expansion of `impl_Display!`
...
562 |       impl_Display!(i8, u8, i16, u16, i32, u32, i64, u64, isize, usize; as u64 into display_u64);
    |       ------------------------------------------------------------------------------------------ in this macro invocation

For more information about this error, try `rustc --explain E0614`.
[RUSTC-TIMING] core test:false 24.199
error: could not compile `core` (lib) due to 5 previous errors

[tgross35]

Huh, does it get &T sometimes and just T others?

You can check that config locally with rust.std-features = ["optimize_for_size"] in your bootstrap.toml