char to_digit: avoid unnecessary casts to u64

[Marcondiro]

Hello,
in the char::to_digit method there are a few as u64 casts that are not strictly necessary.
I assume that the reason behind these casts is to avoid possible overflows in the + 10 add.

This PR removes the aforementioned casts, avoiding the overflow issue by slightly modifying the ASCII letter to int mapping.

Thanks,
Happy new year.

[rustbot]

r? @jhpratt

rustbot has assigned @jhpratt.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[jhpratt]

A quick look shows that the assembly is different as expected, so I'm going to kick off a perf run to verify things. If anything it should be slightly faster, but I know better than to predict perf blindly.

@bors try @rust-timer queue

[bors]

⌛ Trying commit aa685bc with merge 3757ef29634d5acf6b6e0cbf46e69cd7de2481cd...

[klensy]

cc @programmerjake #132709

[bors]

☀️ Try build successful - checks-actions
Build commit: 3757ef2 (3757ef29634d5acf6b6e0cbf46e69cd7de2481cd)

[rust-timer]

Finished benchmarking commit (3757ef2): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

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

Instruction count

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

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

Max RSS (memory usage)

Results (primary 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.

	mean	range	count
Regressions ❌ (primary)	6.0%	[2.0%, 12.6%]	3
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-4.4%	[-8.4%, -2.3%]	4
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[-8.4%, 12.6%]	7

Cycles

Results (secondary -4.6%)

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.

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

Binary size

Results (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.

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.3%]	4
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.1%	[-0.1%, -0.0%]	6
Improvements ✅ (secondary)	-0.0%	[-0.0%, -0.0%]	15
All ❌✅ (primary)	0.0%	[-0.1%, 0.3%]	10

Bootstrap: 762.584s -> 761.845s (-0.10%)
Artifact size: 325.56 MiB -> 325.49 MiB (-0.02%)

[jhpratt]

Regressions seem to be noise, with there being minor improvements in other (less reliable) metrics. Even if it were neutral that would be sufficient for me.

Thanks!

@bors r+

[bors]

📌 Commit aa685bc has been approved by jhpratt

It is now in the queue for this repository.

[programmerjake]

does perf actually run the to_digit benchmarks? i didn't see them in the perf results...

[jhpratt]

@bors r-

Let's discuss on Zulip.

[klensy]

does perf actually run the to_digit benchmarks? i didn't see them in the perf results...

perf runs only on crates listed on page, smaller benchmarks from library not benched that way.

[programmerjake]

I manually ran the to_digit benchmarks on my AMD Ryzen 7950X:

for b in master pull-34969; do git switch "$b"; for i in {1..3}; do RUST_BACKTRACE=1 ./x.py bench library/core -- to_digit > "$b".txt; done; done
diff -U3 master.txt pull-34969.txt

master is at d117b7f, pull-34969 is at aa685bc

--- master.txt  2024-12-31 16:52:47.377514215 -0800
+++ pull-34969.txt      2024-12-31 16:54:43.787387531 -0800
@@ -11,15 +11,15 @@
 bbbbb

 benchmarks:
-    char::methods::bench_to_digit_radix_10   2856.63ns/iter  +/- 20.97
-    char::methods::bench_to_digit_radix_16   2845.83ns/iter  +/- 13.06
-    char::methods::bench_to_digit_radix_2    2843.90ns/iter   +/- 9.80
-    char::methods::bench_to_digit_radix_36   9652.53ns/iter  +/- 54.87
-    char::methods::bench_to_digit_radix_var 15246.25ns/iter +/- 283.04
+    char::methods::bench_to_digit_radix_10   2882.53ns/iter  +/- 9.13
+    char::methods::bench_to_digit_radix_16   2885.77ns/iter  +/- 5.28
+    char::methods::bench_to_digit_radix_2    2882.67ns/iter  +/- 3.06
+    char::methods::bench_to_digit_radix_36   9654.94ns/iter +/- 38.77
+    char::methods::bench_to_digit_radix_var 14723.90ns/iter +/- 41.44

-test result: ok. 0 passed; 0 failed; 0 ignored; 5 measured; 514 filtered out; finished in 1.24s
+test result: ok. 0 passed; 0 failed; 0 ignored; 5 measured; 514 filtered out; finished in 5.39s

-       finished in 1.255 seconds
+       finished in 5.413 seconds

[programmerjake]

ran it another time and the results reversed for radix 2, 10, and 16:

-    char::methods::bench_to_digit_radix_10   2897.59ns/iter +/- 10.50
-    char::methods::bench_to_digit_radix_16   2884.83ns/iter  +/- 6.94
-    char::methods::bench_to_digit_radix_2    2883.97ns/iter +/- 11.17
-    char::methods::bench_to_digit_radix_36   9758.82ns/iter  +/- 5.09
-    char::methods::bench_to_digit_radix_var 15222.17ns/iter +/- 27.43
+    char::methods::bench_to_digit_radix_10   2843.65ns/iter +/- 13.94
+    char::methods::bench_to_digit_radix_16   2853.59ns/iter +/- 17.59
+    char::methods::bench_to_digit_radix_2    2850.38ns/iter +/- 15.10
+    char::methods::bench_to_digit_radix_36   9544.87ns/iter +/- 58.91
+    char::methods::bench_to_digit_radix_var 14527.29ns/iter +/- 38.01

so looks like just measurement noise to me for those and a distinct improvement for the var case, so r+ from me, especially since this PR should be substantially faster for arches without native 64-bit arithmetic.

[jhpratt]

Looks neutral for the common case and an improvement for the variable case, then. A 1%ish difference in either direction is quite small and difficult to detect.

@bors r=jhpratt,programmerjake

[bors]

📌 Commit aa685bc has been approved by jhpratt,programmerjake

It is now in the queue for this repository.

[bors]

⌛ Testing commit aa685bc with merge eeeff9a...

[bors]

☀️ Test successful - checks-actions
Approved by: jhpratt,programmerjake
Pushing eeeff9a to master...

[rust-timer]

Finished benchmarking commit (eeeff9a): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary -0.6%, secondary -2.6%)

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.

	mean	range	count
Regressions ❌ (primary)	7.7%	[2.3%, 13.1%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-6.2%	[-9.7%, -3.9%]	3
Improvements ✅ (secondary)	-2.6%	[-2.6%, -2.6%]	1
All ❌✅ (primary)	-0.6%	[-9.7%, 13.1%]	5

Cycles

Results (secondary -2.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.

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

Binary size

Results (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.

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.3%]	4
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.1%	[-0.1%, -0.0%]	6
Improvements ✅ (secondary)	-0.0%	[-0.0%, -0.0%]	20
All ❌✅ (primary)	0.0%	[-0.1%, 0.3%]	10

Bootstrap: 762.453s -> 760.671s (-0.23%)
Artifact size: 325.56 MiB -> 325.55 MiB (-0.00%)