Skip to content
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

Using fast_float library for faster parsing of 64 decimal strings. #11884

Open
wants to merge 4 commits into
base: unstable
Choose a base branch
from

Conversation

filipecosta90
Copy link
Contributor

@filipecosta90 filipecosta90 commented Mar 6, 2023

Fixes #8825

We're using the fast_float library1 in our (compiled-in) floating-point fast_float_strtod implementation for faster and more portable parsing of 64 decimal strings.

The single file fast_float.h is an amalgamation of the entire library, which can be (re)generated with the amalgamate.py script (from the fast_float repository) via the command:

python3 ./script/amalgamate.py --license=MIT > $REDIS_SRC/deps/fast_float/fast_float.h

The used commit from fast_float library was the one from https://github.com/fastfloat/fast_float/releases/tag/v3.10.1

Still TODO:

…trtod implementation for faster and more portable parsing of 64 decimal strings.
@lemire
Copy link

lemire commented Mar 7, 2023

Upgrading to 3.10.1 might be better: https://github.com/fastfloat/fast_float/releases/tag/v3.10.1

* in the number is put in *ENDPTR. */
extern "C" double fast_float_strtod(const char *nptr, char **endptr) {
double result;
auto answer = fast_float::from_chars(nptr, nptr + strlen(nptr), result);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to catch potential errors, you ought to add..

 if(answer.ec != std::errc()) { /* do something */ }

(Note that the library follows the C++ standard so that's how we handle errors. We did not design this approach.)

@lemire
Copy link

lemire commented Mar 7, 2023

This looks simple enough. My main concern is that you do not check for the error field after parsing. If you only rarely invoke strtod, then switching to the canonical fast_float API might be simpler. Whether this matters for your application, I do not know.

Note that fast_float relies on the C++17 standard API, so adopting the fast_float API is not akin to vendor lockin... it is a standard!!!

@filipecosta90 filipecosta90 added action:run-benchmark Triggers the benchmark suite for this Pull Request and removed action:run-benchmark Triggers the benchmark suite for this Pull Request labels Mar 13, 2023
@lemire
Copy link

lemire commented Aug 22, 2023

Note that the fast_float library has been adopted by webkit (Apple Safari) and other important systems such as GCC.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action:run-benchmark Triggers the benchmark suite for this Pull Request
Projects
None yet
3 participants