-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Rust Float-Parsing Algorithms to use the Eisel-Lemire algorithm. #86761
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @scottmcm (or someone else) soon. Please see the contribution instructions for more information. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
#31109 |
I expect that this PR would fix #31407 |
AsciiStr I've updated AsciiStr to use the slice primitive internally, and also deduplicated a lot of code. Name Changes I've also updated numerous names for methods in common.rs to better reflect safety/unsafety. For example, Testing A few UI regression tests and core regression tests have been added, to address #31109 and #31407, while a few UI tests have been deleted (since they are no longer relevant). Deleted UI Tests Added UI Tests // run-pass
// ignore-tidy-linelength
// Regression test for #31109 and #31407.
// This test should compile and produce expected output.
pub fn main() {
let n: f64 = 0.3333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333;
assert_eq!(n.to_bits(), 0x3fd5555555555555);
let n: f64 = 1234567890123456789012345678901234567890e-340;
assert_eq!(n.to_bits(), 0x1752a64e34ba0d3);
} Added Core Tests #[test]
fn issue31109() {
// Regression test for #31109.
// Ensure the test produces a valid float with the expected bit pattern.
let p = dec2flt::<f64>(
"0.3333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333",
);
assert_eq!(p.map(|x| x.to_bits()), Ok(0x3fd5555555555555));
}
#[test]
fn issue31407() {
// Regression test for #31407.
// Ensure the test produces a valid float with the expected bit pattern.
let p = dec2flt::<f64>("1234567890123456789012345678901234567890e-340");
assert_eq!(p.map(|x| x.to_bits()), Ok(0x1752a64e34ba0d3));
} A few things, however, I am unsure about, and maybe @est31, @estebank, or @jyn514 might help enlighten me on this: --- a/compiler/rustc_mir_build/src/thir/cx/mod.rs
+++ b/compiler/rustc_mir_build/src/thir/cx/mod.rs
@@ -67,12 +67,6 @@ fn new(tcx: TyCtxt<'tcx>, def: ty::WithOptConstParam<LocalDefId>) -> Cx<'tcx> {
match self.tcx.at(sp).lit_to_const(LitToConstInput { lit, ty, neg }) {
Ok(c) => c,
- Err(LitToConstError::UnparseableFloat) => {
- // FIXME(#31407) this is only necessary because float parsing is buggy
- self.tcx.sess.span_err(sp, "could not evaluate float literal (see issue #31407)");
- // create a dummy value and continue compiling
- self.tcx.const_error(ty)
- }
--- a/compiler/rustc_mir_build/src/thir/constant.rs
+++ b/compiler/rustc_mir_build/src/thir/constant.rs
@@ -47,7 +47,7 @@
trunc(if neg { (*n as i128).overflowing_neg().0 as u128 } else { *n })?
}
(ast::LitKind::Float(n, _), ty::Float(fty)) => {
- parse_float(*n, *fty, neg).map_err(|_| LitToConstError::UnparseableFloat)?
+ parse_float(*n, *fty, neg).map_err(|_| LitToConstError::Reported)?
}
--- a/compiler/rustc_mir_build/src/thir/pattern/mod.rs
+++ b/compiler/rustc_mir_build/src/thir/pattern/mod.rs
@@ -563,10 +562,6 @@ fn lower_lit(&mut self, expr: &'tcx hir::Expr<'tcx>) -> PatKind<'tcx> {
LitToConstInput { lit: &lit.node, ty: self.typeck_results.expr_ty(expr), neg };
match self.tcx.at(expr.span).lit_to_const(lit_input) {
Ok(val) => *self.const_to_pat(val, expr.hir_id, lit.span, false).kind,
- Err(LitToConstError::UnparseableFloat) => {
- self.errors.push(PatternError::FloatBug);
- PatKind::Wild
- }
Err(LitToConstError::Reported) => PatKind::Wild,
Err(LitToConstError::TypeError) => bug!("lower_lit: had type error"),
} Likewise, --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs
+++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs
@@ -84,7 +84,7 @@ fn visit_param(&mut self, param: &'tcx hir::Param<'tcx>) {
}
impl PatCtxt<'_, '_> {
- fn report_inlining_errors(&self, pat_span: Span) {
+ fn report_inlining_errors(&self) {
for error in &self.errors {
match *error {
PatternError::StaticInPattern(span) => {
@@ -96,14 +96,6 @@ fn report_inlining_errors(&self, pat_span: Span) {
PatternError::ConstParamInPattern(span) => {
self.span_e0158(span, "const parameters cannot be referenced in patterns")
}
- PatternError::FloatBug => {
- // FIXME(#31407) this is only necessary because float parsing is buggy
- rustc_middle::mir::interpret::struct_error(
- self.tcx.at(pat_span),
- "could not evaluate float literal (see issue #31407)",
- )
- .emit();
- }
--- a/compiler/rustc_mir_build/src/thir/pattern/mod.rs
+++ b/compiler/rustc_mir_build/src/thir/pattern/mod.rs
@@ -31,7 +31,6 @@
AssocConstInPattern(Span),
ConstParamInPattern(Span),
StaticInPattern(Span),
- FloatBug,
NonConstPath(Span),
} The major question I have is:
Questions 2 and 3 refers to parse_float here. Update:
|
@Alexhuszagh fixing bugs does not require approval from t-lang. I don't know about the other questions but it's ok to make changes as follow-ups if this one gets too large. |
Ok, if there's any other changes required (or this PR is too large to adequately review), I'm more than happy to break it into multiple PRs. Let me know, and I'll gladly make the changes as is seen fit. My goal here is to make (what is obviously a fairly large PR with some complex logic) as easy and simple to understand (and audit) as is possible. |
This comment has been minimized.
This comment has been minimized.
The section in the PR description should be updated to use github's issue closing keywords for the fixed issues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got about halfway through and ran out of time; I will probably have similar comments for dec2flt::slow
though.
In general, you have a lot of unsafe here, mostly related to indexing. I tried to review it all but I'm sure I missed some. Have you benchmarked it without the unsafe indexing? It would make me feel better if this had more bounds checks ...
You should be able to fix the test failures with |
I have not, I'm going to try to remove a lot of the unsafe now, and then benchmark it vs. the original implementation. I was trying to remove some unsafe, and then work on making the rest easy to audit, but obviously, that has its own limitations (fast-float-rust is littered with unsafe code that doesn't need to be unsafe and deceptively marked as safe, but the code is well fuzzed). I'm going to do some extensive assembly checks to ensure with decent optimization levels that everything looks similar, and then check benchmarks to ensure that in the macro sense, nothing changes. |
In which sense did the old code rely on UB? |
I meant the old code in fast-float-rust, not in Rust core. There was a function or two (with PRs currently waiting approval) that rely on UB. I've updated this in case it wasn't clear. |
Oh I see, thanks. Once this lands, Miri will also test it. You can try it yourself with the scripts and instructions at https://github.com/RalfJung/miri-test-libstd. If you looked at the float parsing tests i libcore you probably saw the |
Pkgsrc changes: * Bump bootstrap kit version to 1.55.0. * Adjust patches as needed, some no longer apply (so removed) * Update checksum adjustments. * Avoid rust-llvm on SunOS * Optionally build docs * Remove reference to closed/old PR#54621 Upstream changes: Version 1.56.1 (2021-11-01) =========================== - New lints to detect the presence of bidirectional-override Unicode codepoints in the compiled source code ([CVE-2021-42574]) [CVE-2021-42574]: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-42574 Version 1.56.0 (2021-10-21) ======================== Language -------- - [The 2021 Edition is now stable.][rust#88100] See [the edition guide][rust-2021-edition-guide] for more details. - [The pattern in `binding @ pattern` can now also introduce new bindings.] [rust#85305] - [Union field access is permitted in `const fn`.][rust#85769] [rust-2021-edition-guide]: https://doc.rust-lang.org/nightly/edition-guide/rust-2021/index.html Compiler -------- - [Upgrade to LLVM 13.][rust#87570] - [Support memory, address, and thread sanitizers on aarch64-unknown-freebsd.] [rust#88023] - [Allow specifying a deployment target version for all iOS targets][rust#87699] - [Warnings can be forced on with `--force-warn`.][rust#87472] This feature is primarily intended for usage by `cargo fix`, rather than end users. - [Promote `aarch64-apple-ios-sim` to Tier 2\*.][rust#87760] - [Add `powerpc-unknown-freebsd` at Tier 3\*.][rust#87370] - [Add `riscv32imc-esp-espidf` at Tier 3\*.][rust#87666] \* Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Allow writing of incomplete UTF-8 sequences via stdout/stderr on Windows.] [rust#83342] The Windows console still requires valid Unicode, but this change allows splitting a UTF-8 character across multiple write calls. This allows, for instance, programs that just read and write data buffers (e.g. copying a file to stdout) without regard for Unicode or character boundaries. - [Prefer `AtomicU{64,128}` over Mutex for Instant backsliding protection.] [rust#83093] For this use case, atomics scale much better under contention. - [Implement `Extend<(A, B)>` for `(Extend<A>, Extend<B>)`][rust#85835] - [impl Default, Copy, Clone for std::io::Sink and std::io::Empty][rust#86744] - [`impl From<[(K, V); N]>` for all collections.][rust#84111] - [Remove `P: Unpin` bound on impl Future for Pin.][rust#81363] - [Treat invalid environment variable names as non-existent.][rust#86183] Previously, the environment functions would panic if given a variable name with an internal null character or equal sign (`=`). Now, these functions will just treat such names as non-existent variables, since the OS cannot represent the existence of a variable with such a name. Stabilised APIs --------------- - [`std::os::unix::fs::chroot`] - [`UnsafeCell::raw_get`] - [`BufWriter::into_parts`] - [`core::panic::{UnwindSafe, RefUnwindSafe, AssertUnwindSafe}`] These APIs were previously stable in `std`, but are now also available in `core`. - [`Vec::shrink_to`] - [`String::shrink_to`] - [`OsString::shrink_to`] - [`PathBuf::shrink_to`] - [`BinaryHeap::shrink_to`] - [`VecDeque::shrink_to`] - [`HashMap::shrink_to`] - [`HashSet::shrink_to`] These APIs are now usable in const contexts: - [`std::mem::transmute`] - [`[T]::first`][`slice::first`] - [`[T]::split_first`][`slice::split_first`] - [`[T]::last`][`slice::last`] - [`[T]::split_last`][`slice::split_last`] Cargo ----- - [Cargo supports specifying a minimum supported Rust version in Cargo.toml.] [`rust-version`] This has no effect at present on dependency version selection. We encourage crates to specify their minimum supported Rust version, and we encourage CI systems that support Rust code to include a crate's specified minimum version in the text matrix for that crate by default. Compatibility notes ------------------- - [Update to new argument parsing rules on Windows.][rust#87580] This adjusts Rust's standard library to match the behavior of the standard libraries for C/C++. The rules have changed slightly over time, and this PR brings us to the latest set of rules (changed in 2008). - [Disallow the aapcs calling convention on aarch64][rust#88399] This was already not supported by LLVM; this change surfaces this lack of support with a better error message. - [Make `SEMICOLON_IN_EXPRESSIONS_FROM_MACROS` warn by default][rust#87385] - [Warn when an escaped newline skips multiple lines.][rust#87671] - [Calls to `libc::getpid` / `std::process::id` from `Command::pre_exec` may return different values on glibc <= 2.24.][rust#81825] Rust now invokes the `clone3` system call directly, when available, to use new functionality available via that system call. Older versions of glibc cache the result of `getpid`, and only update that cache when calling glibc's clone/fork functions, so a direct system call bypasses that cache update. glibc 2.25 and newer no longer cache `getpid` for exactly this reason. Internal changes ---------------- These changes provide no direct user facing benefits, but represent significant improvements to the internals and overall performance of rustc and related tools. - [LLVM is compiled with PGO in published x86_64-unknown-linux-gnu artifacts.] [rust#88069] This improves the performance of most Rust builds. - [Unify representation of macros in internal data structures.][rust#88019] This change fixes a host of bugs with the handling of macros by the compiler, as well as rustdoc. [`std::os::unix::fs::chroot`]: https://doc.rust-lang.org/stable/std/os/unix/fs/fn.chroot.html [`Iterator::intersperse`]: https://doc.rust-lang.org/stable/std/iter/trait.Iterator.html#method.intersperse [`Iterator::intersperse_with`]: https://doc.rust-lang.org/stable/std/iter/trait.Iterator.html#method.intersperse [`UnsafeCell::raw_get`]: https://doc.rust-lang.org/stable/std/cell/struct.UnsafeCell.html#method.raw_get [`BufWriter::into_parts`]: https://doc.rust-lang.org/stable/std/io/struct.BufWriter.html#method.into_parts [`core::panic::{UnwindSafe, RefUnwindSafe, AssertUnwindSafe}`]: rust-lang/rust#84662 [`Vec::shrink_to`]: https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#method.shrink_to [`String::shrink_to`]: https://doc.rust-lang.org/stable/std/string/struct.String.html#method.shrink_to [`OsString::shrink_to`]: https://doc.rust-lang.org/stable/std/ffi/struct.OsString.html#method.shrink_to [`PathBuf::shrink_to`]: https://doc.rust-lang.org/stable/std/path/struct.PathBuf.html#method.shrink_to [`BinaryHeap::shrink_to`]: https://doc.rust-lang.org/stable/std/collections/struct.BinaryHeap.html#method.shrink_to [`VecDeque::shrink_to`]: https://doc.rust-lang.org/stable/std/collections/struct.VecDeque.html#method.shrink_to [`HashMap::shrink_to`]: https://doc.rust-lang.org/stable/std/collections/hash_map/struct.HashMap.html#method.shrink_to [`HashSet::shrink_to`]: https://doc.rust-lang.org/stable/std/collections/hash_set/struct.HashSet.html#method.shrink_to [`std::mem::transmute`]: https://doc.rust-lang.org/stable/std/mem/fn.transmute.html [`slice::first`]: https://doc.rust-lang.org/stable/std/primitive.slice.html#method.first [`slice::split_first`]: https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_first [`slice::last`]: https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last [`slice::split_last`]: https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_last [`rust-version`]: https://doc.rust-lang.org/nightly/cargo/reference/manifest.html#the-rust-version-field [rust#87671]: rust-lang/rust#87671 [rust#86183]: rust-lang/rust#86183 [rust#87385]: rust-lang/rust#87385 [rust#88100]: rust-lang/rust#88100 [rust#86860]: rust-lang/rust#86860 [rust#84039]: rust-lang/rust#84039 [rust#86492]: rust-lang/rust#86492 [rust#88363]: rust-lang/rust#88363 [rust#85305]: rust-lang/rust#85305 [rust#87832]: rust-lang/rust#87832 [rust#88069]: rust-lang/rust#88069 [rust#87472]: rust-lang/rust#87472 [rust#87699]: rust-lang/rust#87699 [rust#87570]: rust-lang/rust#87570 [rust#88023]: rust-lang/rust#88023 [rust#87760]: rust-lang/rust#87760 [rust#87370]: rust-lang/rust#87370 [rust#87580]: rust-lang/rust#87580 [rust#83342]: rust-lang/rust#83342 [rust#83093]: rust-lang/rust#83093 [rust#88177]: rust-lang/rust#88177 [rust#88548]: rust-lang/rust#88548 [rust#88551]: rust-lang/rust#88551 [rust#88299]: rust-lang/rust#88299 [rust#88220]: rust-lang/rust#88220 [rust#85835]: rust-lang/rust#85835 [rust#86879]: rust-lang/rust#86879 [rust#86744]: rust-lang/rust#86744 [rust#84662]: rust-lang/rust#84662 [rust#86593]: rust-lang/rust#86593 [rust#81050]: rust-lang/rust#81050 [rust#81363]: rust-lang/rust#81363 [rust#84111]: rust-lang/rust#84111 [rust#85769]: rust-lang/rust#85769 (comment) [rust#88490]: rust-lang/rust#88490 [rust#88269]: rust-lang/rust#88269 [rust#84176]: rust-lang/rust#84176 [rust#88399]: rust-lang/rust#88399 [rust#88227]: rust-lang/rust#88227 [rust#88200]: rust-lang/rust#88200 [rust#82776]: rust-lang/rust#82776 [rust#88077]: rust-lang/rust#88077 [rust#87728]: rust-lang/rust#87728 [rust#87050]: rust-lang/rust#87050 [rust#87619]: rust-lang/rust#87619 [rust#81825]: rust-lang/rust#81825 (comment) [rust#88019]: rust-lang/rust#88019 [rust#87666]: rust-lang/rust#87666 Version 1.55.0 (2021-09-09) ============================ Language -------- - [You can now write open "from" range patterns (`X..`), which will start at `X` and will end at the maximum value of the integer.][83918] - [You can now explicitly import the prelude of different editions through `std::prelude` (e.g. `use std::prelude::rust_2021::*;`).][86294] Compiler -------- - [Added tier 3\* support for `powerpc64le-unknown-freebsd`.][83572] \* Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Updated std's float parsing to use the Eisel-Lemire algorithm.][86761] These improvements should in general provide faster string parsing of floats, no longer reject certain valid floating point values, and reduce the produced code size for non-stripped artifacts. - [`string::Drain` now implements `AsRef<str>` and `AsRef<[u8]>`.][86858] Stabilised APIs --------------- - [`Bound::cloned`] - [`Drain::as_str`] - [`IntoInnerError::into_error`] - [`IntoInnerError::into_parts`] - [`MaybeUninit::assume_init_mut`] - [`MaybeUninit::assume_init_ref`] - [`MaybeUninit::write`] - [`array::map`] - [`ops::ControlFlow`] - [`x86::_bittest`] - [`x86::_bittestandcomplement`] - [`x86::_bittestandreset`] - [`x86::_bittestandset`] - [`x86_64::_bittest64`] - [`x86_64::_bittestandcomplement64`] - [`x86_64::_bittestandreset64`] - [`x86_64::_bittestandset64`] The following previously stable functions are now `const`. - [`str::from_utf8_unchecked`] Cargo ----- - [Cargo will now deduplicate compiler diagnostics to the terminal when invoking rustc in parallel such as when using `cargo test`.][cargo/9675] - [The package definition in `cargo metadata` now includes the `"default_run"` field from the manifest.][cargo/9550] - [Added `cargo d` as an alias for `cargo doc`.][cargo/9680] - [Added `{lib}` as formatting option for `cargo tree` to print the `"lib_name"` of packages.][cargo/9663] Rustdoc ------- - [Added "Go to item on exact match" search option.][85876] - [The "Implementors" section on traits no longer shows redundant method definitions.][85970] - [Trait implementations are toggled open by default.][86260] This should make the implementations more searchable by tools like `CTRL+F` in your browser. - [Intra-doc links should now correctly resolve associated items (e.g. methods) through type aliases.][86334] - [Traits which are marked with `#[doc(hidden)]` will no longer appear in the "Trait Implementations" section.][86513] Compatibility Notes ------------------- - [std functions that return an `io::Error` will no longer use the `ErrorKind::Other` variant.][85746] This is to better reflect that these kinds of errors could be categorised [into newer more specific `ErrorKind` variants][79965], and that they do not represent a user error. - [Using environment variable names with `process::Command` on Windows now behaves as expected.][85270] Previously using envionment variables with `Command` would cause them to be ASCII-uppercased. - [Rustdoc will now warn on using rustdoc lints that aren't prefixed with `rustdoc::`][86849] [86849]: rust-lang/rust#86849 [86513]: rust-lang/rust#86513 [86334]: rust-lang/rust#86334 [86260]: rust-lang/rust#86260 [85970]: rust-lang/rust#85970 [85876]: rust-lang/rust#85876 [83572]: rust-lang/rust#83572 [86294]: rust-lang/rust#86294 [86858]: rust-lang/rust#86858 [86761]: rust-lang/rust#86761 [85769]: rust-lang/rust#85769 [85746]: rust-lang/rust#85746 [85305]: rust-lang/rust#85305 [85270]: rust-lang/rust#85270 [84111]: rust-lang/rust#84111 [83918]: rust-lang/rust#83918 [79965]: rust-lang/rust#79965 [87370]: rust-lang/rust#87370 [87298]: rust-lang/rust#87298 [cargo/9663]: rust-lang/cargo#9663 [cargo/9675]: rust-lang/cargo#9675 [cargo/9550]: rust-lang/cargo#9550 [cargo/9680]: rust-lang/cargo#9680 [cargo/9663]: rust-lang/cargo#9663 [`array::map`]: https://doc.rust-lang.org/stable/std/primitive.array.html#method.map [`Bound::cloned`]: https://doc.rust-lang.org/stable/std/ops/enum.Bound.html#method.cloned [`Drain::as_str`]: https://doc.rust-lang.org/stable/std/string/struct.Drain.html#method.as_str [`IntoInnerError::into_error`]: https://doc.rust-lang.org/stable/std/io/struct.IntoInnerError.html#method.into_error [`IntoInnerError::into_parts`]: https://doc.rust-lang.org/stable/std/io/struct.IntoInnerError.html#method.into_parts [`MaybeUninit::assume_init_mut`]: https://doc.rust-lang.org/stable/std/mem/union.MaybeUninit.html#method.assume_init_mut [`MaybeUninit::assume_init_ref`]: https://doc.rust-lang.org/stable/std/mem/union.MaybeUninit.html#method.assume_init_ref [`MaybeUninit::write`]: https://doc.rust-lang.org/stable/std/mem/union.MaybeUninit.html#method.write [`Seek::rewind`]: https://doc.rust-lang.org/stable/std/io/trait.Seek.html#method.rewind [`ops::ControlFlow`]: https://doc.rust-lang.org/stable/std/ops/enum.ControlFlow.html [`str::from_utf8_unchecked`]: https://doc.rust-lang.org/stable/std/str/fn.from_utf8_unchecked.html [`x86::_bittest`]: https://doc.rust-lang.org/stable/core/arch/x86/fn._bittest.html [`x86::_bittestandcomplement`]: https://doc.rust-lang.org/stable/core/arch/x86/fn._bittestandcomplement.html [`x86::_bittestandreset`]: https://doc.rust-lang.org/stable/core/arch/x86/fn._bittestandreset.html [`x86::_bittestandset`]: https://doc.rust-lang.org/stable/core/arch/x86/fn._bittestandset.html [`x86_64::_bittest64`]: https://doc.rust-lang.org/stable/core/arch/x86_64/fn._bittest64.html [`x86_64::_bittestandcomplement64`]: https://doc.rust-lang.org/stable/core/arch/x86_64/fn._bittestandcomplement64.html [`x86_64::_bittestandreset64`]: https://doc.rust-lang.org/stable/core/arch/x86_64/fn._bittestandreset64.html [`x86_64::_bittestandset64`]: https://doc.rust-lang.org/stable/core/arch/x86_64/fn._bittestandset64.html
Summary
Rust, although it implements a correct float parser, has major performance issues in float parsing. Even for common floats, the performance can be 3-10x slower than external libraries such as lexical and fast-float-rust.
Recently, major advances in float-parsing algorithms have been developed by Daniel Lemire, along with others, and implement a fast, performant, and correct float parser, with speeds up to 1200 MiB/s on Apple's M1 architecture for the canada dataset, 10x faster than Rust's 130 MiB/s.
In addition, edge-cases in Rust's dec2flt algorithm can lead to over a 1600x slowdown relative to efficient algorithms. This is due to the use of Clinger's correct, but slow AlgorithmM and Bellepheron, which have been improved by faster big-integer algorithms and the Eisel-Lemire algorithm, respectively.
Finally, this algorithm provides substantial improvements in the number of floats the Rust core library can parse. Denormal floats with a large number of digits cannot be parsed, due to use of the
Big32x40
, which simply does not have enough digits to round a float correctly. Using a custom decimal class, with much simpler logic, we can parse all valid decimal strings of any digit count.Solution
This pull request implements the Eisel-Lemire algorithm, modified from fast-float-rust (which is licensed under Apache 2.0/MIT), along with numerous modifications to make it more amenable to inclusion in the Rust core library. The following describes both features in fast-float-rust and improvements in fast-float-rust for inclusion in core.
Documentation
Extensive documentation has been added to ensure the code base may be maintained by others, which explains the algorithms as well as various associated constants and routines. For example, two seemingly magical constants include documentation to describe how they were derived as follows:
This ensures maintainability of the code base.
Improvements for Disguised Fast-Path Cases
The fast path in float parsing algorithms attempts to use native, machine floats to represent both the significant digits and the exponent, which is only possible if both can be exactly represented without rounding. In practice, this means that the significant digits must be 53-bits or less and the then exponent must be in the range
[-22, 22]
(for an f64). This is similar to the existing dec2flt implementation.However, disguised fast-path cases exist, where there are few significant digits and an exponent above the valid range, such as
1.23e25
. In this case, powers-of-10 may be shifted from the exponent to the significant digits, discussed at length in #85198.Digit Parsing Improvements
Typically, integers are parsed from string 1-at-a-time, requiring unnecessary multiplications which can slow down parsing. An approach to parse 8 digits at a time using only 3 multiplications is described in length here. This leads to significant performance improvements, and is implemented for both big and little-endian systems.
Unsafe Changes
Relative to fast-float-rust, this library makes less use of unsafe functionality and clearly documents it. This includes the refactoring and documentation of numerous unsafe methods undesirably marked as safe. The original code would look something like this, which is deceptively marked as safe for unsafe functionality.
The new code clearly documents safety concerns, and does not mark unsafe functionality as safe, leading to better safety guarantees.
This allows us to trivially demonstrate the new implementation of dec2flt is safe.
Inline Annotations Have Been Removed
In the previous implementation of dec2flt, inline annotations exist practically nowhere in the entire module. Therefore, these annotations have been removed, which mostly does not impact performance.
Fixed Correctness Tests
Numerous compile errors in
src/etc/test-float-parse
were present, due to deprecation oftime.clock()
, as well as the crate dependencies withrand
. The tests have therefore been reworked as a crate, and any errors inruntests.py
have been patched.Undefined Behavior
An implementation of
check_len
which relied on undefined behavior (in fast-float-rust) has been refactored, to ensure that the behavior is well-defined. The original code is as follows:And the new implementation is as follows:
Note that this has since been fixed in fast-float-rust.
Inferring Binary Exponents
Rather than explicitly store binary exponents, this new implementation infers them from the decimal exponent, reducing the amount of static storage required. This removes the requirement to store 611 i16s.
Code Size
The code size, for all optimizations, does not considerably change relative to before for stripped builds, however it is significantly smaller prior to stripping the resulting binaries. These binary sizes were calculated on x86_64-unknown-linux-gnu.
new
Using rustc version 1.55.0-dev.
old
Using rustc version 1.53.0-nightly.
Correctness
The dec2flt implementation passes all of Rust's unittests and comprehensive float parsing tests, along with numerous other tests such as Nigel Toa's comprehensive float tests and Hrvoje Abraham strtod_tests. Therefore, it is unlikely that this algorithm will incorrectly round parsed floats.
Issues Addressed
This will fix and close the following issues:
FloatBug
in pattern #68396