Faster fmt::Display of 128-bit integers, without unsafe pointer by pascaldekloe · Pull Request #136594 · rust-lang/rust

Hey, I'm very sorry this PR kept falling off my todo list. I have now had a chance to read through thoroughly and it looks great. Left a few small comments, but with those, a rebase, and a final try job I think this should be good to go.

Any chance you could post local benchmarks as well?

library/core/src/fmt/num.rs

tgross35 · 2025-06-10T17:51:11Z

@bors try @rust-timer queue

rust-timer · 2025-06-10T22:27:50Z

Finished benchmarking commit (f791aa5): comparison URL.

Overall result: ✅ 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
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.4%	[-0.4%, -0.4%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary 0.8%)

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)	8.5%	[8.5%, 8.5%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.8%	[-2.9%, -1.0%]	3
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary 2.7%)

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)	2.7%	[2.7%, 2.7%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

Results (primary -0.0%)

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)	-	-	0
Improvements ✅ (primary)	-0.0%	[-0.0%, -0.0%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.0%	[-0.0%, -0.0%]	1

Bootstrap: 754.321s -> 755.157s (0.11%)
Artifact size: 372.15 MiB -> 372.14 MiB (-0.00%)

tgross35 · 2025-06-11T01:24:08Z

One final comment request #136594 (comment) and it would be good if you could post benchmarks and/or an asm diff. After that, r=me

pascaldekloe · 2025-06-11T12:42:52Z

The following was measured locally on Apple M1 @tgross35.

Master

    fmt::write_u128_max                                                   51.44ns/iter      +/- 0.44
    fmt::write_u128_min                                                   30.63ns/iter      +/- 0.92
    fmt::write_u64_max                                                    36.00ns/iter      +/- 0.55
    fmt::write_u64_min                                                    27.72ns/iter      +/- 0.20
    fmt::write_u8_max                                                     28.98ns/iter      +/- 0.34
    fmt::write_u8_min                                                     27.72ns/iter      +/- 0.38

Change

    fmt::write_u128_max                                                   46.65ns/iter     +/- 0.39
    fmt::write_u128_min                                                   26.77ns/iter     +/- 0.58
    fmt::write_u64_max                                                    35.68ns/iter     +/- 0.39
    fmt::write_u64_min                                                    27.72ns/iter     +/- 0.75
    fmt::write_u8_max                                                     28.71ns/iter     +/- 0.08
    fmt::write_u8_min                                                     27.39ns/iter     +/- 0.32

tgross35 · 2025-06-11T18:27:33Z

Thank you for all the work here!

@bors r+

bors · 2025-06-11T18:27:35Z

📌 Commit 042a271 has been approved by tgross35

It is now in the queue for this repository.

bors · 2025-06-12T01:02:58Z

⌛ Testing commit 042a271 with merge 1434630...

bors · 2025-06-12T04:04:53Z

☀️ Test successful - checks-actions
Approved by: tgross35
Pushing 1434630 to master...

github-actions · 2025-06-12T04:08:09Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 31d0d21 (parent) -> 1434630 (this PR)

Test differences

Show 5 test diffs

Stage 1

fmt::num::test_format_int: pass -> [missing] (J0)
fmt::num::test_format_int_limits: [missing] -> pass (J0)
fmt::num::test_format_int_misc: [missing] -> pass (J0)
fmt::num::test_format_int_one: [missing] -> pass (J0)
fmt::num::test_format_int_twos_complement: pass -> [missing] (J0)

Job group index

J0: aarch64-apple, aarch64-gnu, arm-android, armhf-gnu, dist-i586-gnu-i586-i686-musl, i686-gnu-1, i686-gnu-nopt-1, i686-msvc-1, mingw-check-2, test-various, x86_64-apple-1, x86_64-gnu, x86_64-gnu-aux, x86_64-gnu-llvm-19-2, x86_64-gnu-llvm-19-3, x86_64-gnu-llvm-20-2, x86_64-gnu-llvm-20-3, x86_64-gnu-nopt, x86_64-gnu-stable, x86_64-mingw-1, x86_64-msvc-1

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 14346303d760027e53214e705109a62c0f00b214 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

x86_64-apple-1: 7215.7s -> 9403.1s (30.3%)
dist-x86_64-apple: 8674.0s -> 10174.7s (17.3%)
mingw-check-1: 1656.4s -> 1911.2s (15.4%)
aarch64-apple: 5855.4s -> 5214.8s (-10.9%)
i686-gnu-1: 7603.4s -> 8357.3s (9.9%)
x86_64-gnu-llvm-20-1: 3304.6s -> 3627.0s (9.8%)
i686-gnu-2: 5685.3s -> 6230.8s (9.6%)
armhf-gnu: 4535.6s -> 4966.5s (9.5%)
x86_64-rust-for-linux: 2648.9s -> 2853.9s (7.7%)
x86_64-gnu-debug: 5580.8s -> 5992.3s (7.4%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2025-06-12T12:04:31Z

Finished benchmarking commit (1434630): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -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.2%	[0.1%, 0.3%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary 1.5%)

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)	1.5%	[1.5%, 1.5%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary 6.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)	6.6%	[6.6%, 6.6%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

Results (primary -0.0%)

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)	-	-	0
Improvements ✅ (primary)	-0.0%	[-0.0%, -0.0%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.0%	[-0.0%, -0.0%]	1

Bootstrap: 754.746s -> 755.927s (0.16%)
Artifact size: 372.10 MiB -> 372.13 MiB (0.01%)

Faster fmt::Display of 128-bit integers, without unsafe pointer In followup of rust-lang#135265, hereby the 128-bit part. * Batches per 16 instead of 19 digits * Buffer access as array insteaf of unsafe pointer * Added test coverage for i128 and u128 r? tgross35 ChrisDenton

single buffer for exponent fmt of integers No need for fragmented buffers when formatting. ``` orig.txt: fmt::write_i128_exp 143.39ns/iter +/- 0.32 orig.txt: fmt::write_i64_exp 68.72ns/iter +/- 0.03 new.txt: fmt::write_i128_exp 138.29ns/iter +/- 0.50 new.txt: fmt::write_i64_exp 58.93ns/iter +/- 4.62 ``` This patch fully eliminates unsafe pointer use (after rust-lang#135265 and rust-lang#136594). r? libs

Rollup merge of #145940 - pascaldekloe:int-exp-tune, r=tgross35 single buffer for exponent fmt of integers No need for fragmented buffers when formatting. ``` orig.txt: fmt::write_i128_exp 143.39ns/iter +/- 0.32 orig.txt: fmt::write_i64_exp 68.72ns/iter +/- 0.03 new.txt: fmt::write_i128_exp 138.29ns/iter +/- 0.50 new.txt: fmt::write_i64_exp 58.93ns/iter +/- 4.62 ``` This patch fully eliminates unsafe pointer use (after #135265 and #136594). r? libs

single buffer for exponent fmt of integers No need for fragmented buffers when formatting. ``` orig.txt: fmt::write_i128_exp 143.39ns/iter +/- 0.32 orig.txt: fmt::write_i64_exp 68.72ns/iter +/- 0.03 new.txt: fmt::write_i128_exp 138.29ns/iter +/- 0.50 new.txt: fmt::write_i64_exp 58.93ns/iter +/- 4.62 ``` This patch fully eliminates unsafe pointer use (after rust-lang/rust#135265 and rust-lang/rust#136594). r? libs

single buffer for exponent fmt of integers No need for fragmented buffers when formatting. ``` orig.txt: fmt::write_i128_exp 143.39ns/iter +/- 0.32 orig.txt: fmt::write_i64_exp 68.72ns/iter +/- 0.03 new.txt: fmt::write_i128_exp 138.29ns/iter +/- 0.50 new.txt: fmt::write_i64_exp 58.93ns/iter +/- 4.62 ``` This patch fully eliminates unsafe pointer use (after rust-lang#135265 and rust-lang#136594). r? libs

single buffer for exponent fmt of integers No need for fragmented buffers when formatting. ``` orig.txt: fmt::write_i128_exp 143.39ns/iter +/- 0.32 orig.txt: fmt::write_i64_exp 68.72ns/iter +/- 0.03 new.txt: fmt::write_i128_exp 138.29ns/iter +/- 0.50 new.txt: fmt::write_i64_exp 58.93ns/iter +/- 4.62 ``` This patch fully eliminates unsafe pointer use (after rust-lang/rust#135265 and rust-lang/rust#136594). r? libs

rustbot assigned tgross35 Feb 5, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 5, 2025

This comment has been minimized.

Sign in to view

pascaldekloe force-pushed the fmt-int128 branch from 658adc6 to 3a09315 Compare February 6, 2025 12:43

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 26, 2025

rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 26, 2025

pascaldekloe force-pushed the fmt-int128 branch from 3a09315 to bfb09cf Compare April 28, 2025 13:48

This comment has been minimized.

Sign in to view

pascaldekloe force-pushed the fmt-int128 branch from bfb09cf to 1ce1b97 Compare April 29, 2025 20:16

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 30, 2025

tgross35 reviewed Jun 10, 2025

View reviewed changes

format integer tests regrouped, min/max coverage and 128-bit coverage

e9ebe25

pascaldekloe force-pushed the fmt-int128 branch from 1ce1b97 to 0483c93 Compare June 10, 2025 17:31

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 10, 2025

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 10, 2025

faster fmt::Display of 128-bit integers, without unsafe pointer

042a271

pascaldekloe force-pushed the fmt-int128 branch from 0483c93 to 042a271 Compare June 11, 2025 12:21

tgross35 approved these changes Jun 11, 2025

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 11, 2025

bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 12, 2025

bors merged commit 1434630 into rust-lang:master Jun 12, 2025
11 checks passed

rustbot added this to the 1.89.0 milestone Jun 12, 2025

bors mentioned this pull request Jun 12, 2025

Use a distinct ToString implementation for u128 and i128 #142294

Merged

hanna-kruppe mentioned this pull request Jul 1, 2025

Implement int_format_into feature #142098

Merged

pascaldekloe mentioned this pull request Aug 27, 2025

single buffer for exponent fmt of integers #145940

Merged

qsctr mentioned this pull request Dec 1, 2025

Update Rust toolchain to 1.91 (nightly-2025-09-14) GaloisInc/mir-json#200

Merged

Uh oh!

Conversation

pascaldekloe commented Feb 5, 2025

Uh oh!

This comment has been minimized.

pascaldekloe commented Feb 5, 2025

Uh oh!

This comment has been minimized.

bors commented Feb 5, 2025

Uh oh!

tgross35 commented Feb 5, 2025

Uh oh!

bors commented Feb 6, 2025

Uh oh!

pascaldekloe commented Feb 6, 2025

Uh oh!

This comment has been minimized.

pascaldekloe commented Feb 7, 2025

Uh oh!

tgross35 commented Apr 26, 2025

Uh oh!

rustbot commented Apr 26, 2025

Uh oh!

This comment has been minimized.

pascaldekloe commented Apr 30, 2025

Uh oh!

bors commented Jun 7, 2025

Uh oh!

tgross35 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tgross35 commented Jun 10, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-timer commented Jun 10, 2025

Overall result: ✅ improvements - no action needed

Uh oh!

tgross35 commented Jun 11, 2025

Uh oh!

pascaldekloe commented Jun 11, 2025

Master

Change

Uh oh!

tgross35 commented Jun 11, 2025

Uh oh!

bors commented Jun 11, 2025

Uh oh!

bors commented Jun 12, 2025

Uh oh!

bors commented Jun 12, 2025

Uh oh!

Uh oh!

github-actions bot commented Jun 12, 2025

Test differences

Stage 1

Job duration changes

Uh oh!

rust-timer commented Jun 12, 2025

Overall result: ❌ regressions - no action needed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants