fmt of non-decimal radix untangled #143730

pascaldekloe · 2025-07-10T12:51:01Z

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 · 2025-07-10T12:58:31Z

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

bors · 2025-07-17T08:43:24Z

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

pascaldekloe · 2025-07-18T13:37:22Z

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

tgross35 · 2025-07-18T19:02:16Z

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 · 2025-07-18T21:14:44Z

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 · 2025-07-18T21:56:23Z

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

library/core/src/fmt/num.rs

library/coretests/benches/fmt.rs

…t output

tgross35

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

library/core/src/fmt/num.rs

* correct buffer size * no trait abstraction * similar to decimal

tgross35 · 2025-08-12T19:06:19Z

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

r=me with that passing

fmt of non-decimal radix untangled try-job: x86_64-gnu-llvm-19-3

rust-bors · 2025-08-12T20:56:47Z

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

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

tgross35 · 2025-08-12T21:00:49Z

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

pascaldekloe · 2025-08-16T21:28:03Z

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

Apparently such changes don't always get picked up by ./x test. I will dig in deeper into why at some point if time permits.

tgross35 · 2025-08-18T18:59:38Z

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

fmt of non-decimal radix untangled try-job: x86_64-gnu-llvm-19-3

rust-bors · 2025-08-18T21:03:13Z

☀️ Try build successful (CI)
Build commit: 054eb81 (054eb81f0211a6054e03c271809d347281601102, parent: 239e8b1b47b34120287ec36b33228c1e177f0c38)

tgross35 · 2025-08-18T23:03:15Z

Thanks for the fix!

@bors r+

Apparently such changes don't always get picked up by ./x test. I will dig in deeper into why at some point if time permits.

Maybe just bring this up the #t-infra/bootstrap Zulip, somebody may know a better idea of why

bors · 2025-08-18T23:03:18Z

📌 Commit 1f77424 has been approved by tgross35

It is now in the queue for this repository.

Noratrieb · 2025-08-19T05:55:44Z

@bors rollup=maybe

…oss35 fmt of non-decimal radix untangled 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

Rollup of 15 pull requests Successful merges: - #139345 (Extend `QueryStability` to handle `IntoIterator` implementations) - #140740 (Add `-Zindirect-branch-cs-prefix`) - #142079 (nll-relate: improve hr opaque types support) - #142938 (implement std::fs::set_permissions_nofollow on unix) - #143730 (fmt of non-decimal radix untangled) - #144767 (Correct some grammar in integer documentation) - #144906 (Require approval from t-infra instead of t-release on tier bumps) - #144983 (Rehome 37 `tests/ui/issues/` tests to other subdirectories under `tests/ui/`) - #145025 (run spellcheck as a tidy extra check in ci) - #145099 (rustc_target: Add the `32s` target feature for LoongArch) - #145166 (suggest using `pub(crate)` for E0364) - #145255 (dec2flt: Provide more valid inputs examples) - #145306 (Add tracing to various miscellaneous functions) - #145336 (Hide docs for `core::unicode`) - #145585 (Miri: fix handling of in-place argument and return place handling) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #143730 - pascaldekloe:fmt-radix-trim, r=tgross35 fmt of non-decimal radix untangled 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

Rollup of 15 pull requests Successful merges: - rust-lang#139345 (Extend `QueryStability` to handle `IntoIterator` implementations) - rust-lang#140740 (Add `-Zindirect-branch-cs-prefix`) - rust-lang#142079 (nll-relate: improve hr opaque types support) - rust-lang#142938 (implement std::fs::set_permissions_nofollow on unix) - rust-lang#143730 (fmt of non-decimal radix untangled) - rust-lang#144767 (Correct some grammar in integer documentation) - rust-lang#144906 (Require approval from t-infra instead of t-release on tier bumps) - rust-lang#144983 (Rehome 37 `tests/ui/issues/` tests to other subdirectories under `tests/ui/`) - rust-lang#145025 (run spellcheck as a tidy extra check in ci) - rust-lang#145099 (rustc_target: Add the `32s` target feature for LoongArch) - rust-lang#145166 (suggest using `pub(crate)` for E0364) - rust-lang#145255 (dec2flt: Provide more valid inputs examples) - rust-lang#145306 (Add tracing to various miscellaneous functions) - rust-lang#145336 (Hide docs for `core::unicode`) - rust-lang#145585 (Miri: fix handling of in-place argument and return place handling) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 15 pull requests Successful merges: - rust-lang/rust#139345 (Extend `QueryStability` to handle `IntoIterator` implementations) - rust-lang/rust#140740 (Add `-Zindirect-branch-cs-prefix`) - rust-lang/rust#142079 (nll-relate: improve hr opaque types support) - rust-lang/rust#142938 (implement std::fs::set_permissions_nofollow on unix) - rust-lang/rust#143730 (fmt of non-decimal radix untangled) - rust-lang/rust#144767 (Correct some grammar in integer documentation) - rust-lang/rust#144906 (Require approval from t-infra instead of t-release on tier bumps) - rust-lang/rust#144983 (Rehome 37 `tests/ui/issues/` tests to other subdirectories under `tests/ui/`) - rust-lang/rust#145025 (run spellcheck as a tidy extra check in ci) - rust-lang/rust#145099 (rustc_target: Add the `32s` target feature for LoongArch) - rust-lang/rust#145166 (suggest using `pub(crate)` for E0364) - rust-lang/rust#145255 (dec2flt: Provide more valid inputs examples) - rust-lang/rust#145306 (Add tracing to various miscellaneous functions) - rust-lang/rust#145336 (Hide docs for `core::unicode`) - rust-lang/rust#145585 (Miri: fix handling of in-place argument and return place handling) r? `@ghost` `@rustbot` modify labels: rollup

…oss35 fmt of non-decimal radix untangled 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

rustbot assigned tgross35 Jul 10, 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 Jul 10, 2025

pascaldekloe force-pushed the fmt-radix-trim branch from 2281143 to d593c6f Compare July 10, 2025 16:05

This comment has been minimized.

Sign in to view

pascaldekloe force-pushed the fmt-radix-trim branch from d593c6f to f0f1c53 Compare July 10, 2025 16:31

This comment has been minimized.

Sign in to view

pascaldekloe force-pushed the fmt-radix-trim branch from f0f1c53 to 76b8c3a Compare July 10, 2025 17:18

pascaldekloe force-pushed the fmt-radix-trim branch from 76b8c3a to 110d35e Compare July 17, 2025 14:05

rustbot assigned ibraheemdev and unassigned tgross35 Jul 18, 2025

tgross35 self-assigned this Jul 18, 2025

pascaldekloe force-pushed the fmt-radix-trim branch from 110d35e to 2c6210d Compare July 22, 2025 14:51

tgross35 reviewed Jul 23, 2025

View reviewed changes

library/core/src/fmt/num.rs Outdated Show resolved Hide resolved

library/core/src/fmt/num.rs Outdated Show resolved Hide resolved

library/core/src/fmt/num.rs Outdated Show resolved Hide resolved

library/coretests/benches/fmt.rs Show resolved Hide resolved

pascaldekloe added 2 commits July 25, 2025 16:02

fmt of non-decimals is always non-negative due to the two's-complemen…

c6ac515

…t output

fmt benchmarks for binary, octal and hex

7704a39

pascaldekloe force-pushed the fmt-radix-trim branch from 2c6210d to 6e3b5ee Compare July 25, 2025 15:03

tgross35 reviewed Aug 1, 2025

View reviewed changes

library/core/src/fmt/num.rs Outdated Show resolved Hide resolved

library/core/src/fmt/num.rs Outdated Show resolved Hide resolved

library/core/src/fmt/num.rs Show resolved Hide resolved

library/core/src/fmt/num.rs Outdated Show resolved Hide resolved

tgross35 unassigned ibraheemdev Aug 1, 2025

pascaldekloe force-pushed the fmt-radix-trim branch from 6e3b5ee to c424403 Compare August 1, 2025 12:59

This comment has been minimized.

Sign in to view

fmt with table lookup for binary, octal and hex

4ad8606

* correct buffer size * no trait abstraction * similar to decimal

pascaldekloe force-pushed the fmt-radix-trim branch from c424403 to d7da6f7 Compare August 2, 2025 11:27

This comment has been minimized.

Sign in to view

pascaldekloe force-pushed the fmt-radix-trim branch from 0291e78 to ca1d99b Compare August 12, 2025 14:23

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Aug 12, 2025

Auto merge of #143730 - pascaldekloe:fmt-radix-trim, r=<try>

59e4e19

fmt of non-decimal radix untangled try-job: x86_64-gnu-llvm-19-3

This comment has been minimized.

Sign in to view

fmt::DisplayInt abstraction obsolete with better macro

1f77424

pascaldekloe force-pushed the fmt-radix-trim branch from ca1d99b to 1f77424 Compare August 16, 2025 16:33

This comment was marked as spam.

Sign in to view

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Aug 18, 2025

Auto merge of #143730 - pascaldekloe:fmt-radix-trim, r=<try>

054eb81

fmt of non-decimal radix untangled try-job: x86_64-gnu-llvm-19-3

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 18, 2025

This was referenced Aug 19, 2025

Rollup of 17 pull requests #145598

Closed

Rollup of 15 pull requests #145599

Merged

bors merged commit 4327e69 into rust-lang:master Aug 19, 2025
11 checks passed

rustbot added this to the 1.91.0 milestone Aug 19, 2025

fmt of non-decimal radix untangled #143730

fmt of non-decimal radix untangled #143730

Uh oh!

Conversation

pascaldekloe commented Jul 10, 2025

Original Performance

Patched Performance

pascaldekloe commented Jul 10, 2025

This comment has been minimized.

This comment has been minimized.

bors commented Jul 17, 2025

pascaldekloe commented Jul 18, 2025

tgross35 commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

tgross35 commented Jul 18, 2025

pascaldekloe commented Jul 18, 2025

tgross35 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tgross35 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

tgross35 commented Aug 12, 2025

This comment has been minimized.

rust-bors bot commented Aug 12, 2025

This comment has been minimized.

tgross35 commented Aug 12, 2025

This comment was marked as spam.

pascaldekloe commented Aug 16, 2025

tgross35 commented Aug 18, 2025

This comment has been minimized.

rust-bors bot commented Aug 18, 2025

tgross35 commented Aug 18, 2025

bors commented Aug 18, 2025

Noratrieb commented Aug 19, 2025

Uh oh!

Labels

9 participants

tgross35 commented Jul 18, 2025 •

edited

Loading