single buffer for exponent fmt of integers

[pascaldekloe]

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

[rustbot]

r? @ibraheemdev

rustbot has assigned @ibraheemdev.
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

[pascaldekloe]

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

Do you want to have another look @tgross35? 🙏 I think we can (and should) format floating points in a single buffer too. If that succeeds then we can drop unstable feature numfmt, package num::fmt. ✨

[tgross35]

Thanks for the ping, sure I'm happy to take the formatting followups here

r? tgross35

[tgross35]

Another great change! Looks mostly good, few small things and doc nits

View changes since this review

[tgross35]

Double checking that optimize for size job. Also don't think perf will show anything but may as well run it

@bors2 try jobs=x86_64-gnu,x86_64-gnu-llvm-19-3
@rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[rust-bors]

☀️ Try build successful (CI)
Build commit: 5fe1474 (5fe147445d224fb3e792d4558c52702727add2fc, parent: 91edc3ebccc4daa46c20a93f4709862376da1fdd)

[rust-timer]

Queued 5fe1474 with parent 91edc3e, future comparison URL.
There are currently 2 preceding artifacts in the queue.
It will probably take at least ~2.9 hours until the benchmark run finishes.

[pascaldekloe]

Another great review! The revision on floating points is starting to work too now. With a little luck we have this piggy washed somewhere next week. 🐷 🧼

[tgross35]

Another great review! The revision on floating points is starting to work too now. With a little luck we have this piggy washed somewhere next week. 🐷 🧼

Feel free to request me for these reviews too, I've worked on those a bit. Very much looking forward to them!

If you're working on these and up for it, one easy thing I've wanted for a little while is to refactor the traits a bit. Basically:

• Rename RawFloat to Float and try to make this only about properties that all floats have, nothing algorithm-specific. Ideal if the names match up with https://github.com/rust-lang/compiler-builtins/blob/a63d089c673aa9397d583c3cef506ad457c5f403/libm/src/math/support/float_traits.rs (obviously we don't need the whole list of constants/values there)
• Get rid of DecodableFloat, it's just MIN_POSITIVE_NORMAL in the above trait
• Introduce traits like LemireExt, GrisuExt, and DragonExt that algorithm-specific functions and constants (not sure all will be needed)

Reasoning here is that f128 probably needs a different algorithm, and some of the algorithm-specific constants on the current RawFloat doesn't really make sense for it. Also should make it easier to try something new like the dragonbox algorithm.

[pascaldekloe]

The Grisu and Dragon implementations use flt2dec::decoder::Decoded as an abstraction, which seems like an elegant solution, except maybe for the 26-byte footprint. What confuses me is the enormous amount of abstractions (and copies) that get introduced before the Decoded extraction.

DecodableFloat is gone indeed. Don't need the Float abstraction either. It seems so much easier to deal with the edge cases by just calling f64::is_* in the macro_rules themselves. Enumerations FullDecoded and Sign are gone too. I'm hoping it all works out without some hidden catch. 🙏

Can you remove the S-waiting-on-author @tgross35?

[tgross35]

Thanks for the updates. I guess the push removed this from the perf queue, but I don't expect to see much there anyway so

@bors r+

Can you remove the S-waiting-on-author @tgross35?

Commenting @rustbot review does this, re-requesting a review should too.

[bors]

📌 Commit 56e95aa has been approved by tgross35

It is now in the queue for this repository.