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

[tgross35]

Let’s sanity check this:

@bors2 try @rust-timer queue

[rust-bors]

⌛ Trying commit d7da6f7 with merge 4984ea3…

To cancel the try build, run the command @bors try cancel.

[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%)

[rustbot]

⚠️ Warning ⚠️

• This PR is based on an upstream commit that is 17 days old.

  It's recommended to update your branch according to the rustc-dev-guide.

[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.