Ping @fcostaoliveira that I believe was the original commit author that introduced the fast path for floats parsing. Would love a review from him. Thanks.

@antirez

Fallback to standard strtod() for complex cases to ensure correctly-rounded results

Do note that the fast_float library provides correctly-rounded results always.

So the C port should not need a fallback.

@antirez

Fallback to standard strtod() for complex cases to ensure correctly-rounded results

Do note that the fast_float library provides correctly-rounded results always.

So the C port should not need a fallback.

The C implementation is not a port of the C++ code, this is why you can see this huge difference in the lines of code needed for the implementation. This code was written with the mandatory specification of just incapsulating the fast path and not caring about the complex edge cases to handle, which are rare, complicated, and that strtod() can handle very well without we having any speed regression.

checking the original benchmarks used to approve fast_float on redis (#11884 (comment)) we see pretty much the same performance between this branch and unstable, making it "safe" performance wise to merge.
Haven't reviewed the code yet, just performance. Will do that until EOD (both me and @paulorsousa )

details on benchmarks on arm-aws-m8g.metal-24xl:

details on benchmarks on x86-aws-m7i.metal-24xl:

Thank you! It is great that the brach is still as fast as before. The new C code only covers the subset that "matters", so it was important to be sure that there were no regression speed. Appreciated!

hi, if possible can you elaborate some of the test case and test data you have on hand?

Hello, I see this is still blocked. Could I ask what I can do in order to put this forward? Thanks.

Is the intention to retain the MIT license? Now that it's pure C, it might be worth pulling into hiredis and phpredis 😄

Any update on this?

The fast_float dependency required C++ (libstdc++) to build Redis.

Thankfully, it does not. You do need at C++ compiler at compile time but you do not need to introduce a dependency on the C++ standard library.

See

#14812

@antirez

Fallback to standard strtod() for complex cases to ensure correctly-rounded results

Do note that the fast_float library provides correctly-rounded results always.
So the C port should not need a fallback.

The C implementation is not a port of the C++ code, this is why you can see this huge difference in the lines of code needed for the implementation. This code was written with the mandatory specification of just incapsulating the fast path and not caring about the complex edge cases to handle, which are rare, complicated, and that strtod() can handle very well without we having any speed regression.

@antirez note that in the current redis code, if fast_float_strtod() fails, it will fallback to strtod(). However, in this PR, there is already a fallback, which means that if it is an illegal double string, we will definitely fall back twice.

e.g.

int string2d(const char *s, size_t slen, double *dp) {
    ...
    *dp = fast_float_strtod(s, &eptr);
    // fallback to strtod
    if (unlikely((size_t)(eptr - (char*)s) != slen)) {
        char *fallback_eptr;
        *dp = strtod(s, &fallback_eptr);
        if ((size_t)(fallback_eptr - (char*)s) != slen) return 0;
    }
    if (unlikely(errno == EINVAL ||
        (errno == ERANGE &&
            (*dp == HUGE_VAL || *dp == -HUGE_VAL || fpclassify(*dp) == FP_ZERO)) ||
        isnan(*dp)))
        return 0;
    return 1;
}

Funny timing, I recently performed a by-hand port of fast_float to plain C99. It lives here:

https://github.com/kolemannix/ffc.h?tab=readme-ov-file#benchmarks

It passes the 'exhaustive' test suite for all 32-bit bit patterns roundtripped to float and double, and is currently benchmarking at (slightly above) the level of the original cpp.

This could be a viable alternative to this PR. I believe my port does not suffer from a few of the issues raised in the comments in this PR (strlen, reparse)

*Edit: we haven't tested the specialized 32-bit architecture code yet; if this is of interest for the redis project, we can focus on that asap

*Edit 2: tests are passing under x64 as of kolemannix/ffc.h#5

We have an initial release of ffc.h published, I'll work a variant of this branch that uses ffc.h instead. I understand if its more than you want, the fallback bigint stuff, if falling back to strtod is satisfactory.

But if that is the main concern, I am happy to add a macro feature that removes the bigint fallback code and either delegates to strdo(d|f) or returns a specific outcome code for that case.

@sundb @fcostaoliveira please could you handle this from this point on? Feel free to do the thing that you believe are more appropriate. I vote for using the implementation I provided that is stripped from everything we don't need. However note that there is an error:

The actual maximum output of the conversion bug is 26 bytes: (18 significant digits + 7 trailing zeros + 1 for - sign).
So the declaration should be:

  // fpconv_dtoa.h
  int fpconv_dtoa(double fp, char dest[26]);

However in all the call sites a larger buffer is used right now, but still.

Thanks

This changes semantics, not just implementation. The new fast_float_strtod() fallback now accepts hex literals (0x1, 0x1p0) for direct callers that previously rejected them, which affects commands like ZCOUNT / ZRANGEBYSCORE / ZREMRANGEBYSCORE / SORT BY. I don’t think Redis generally treats hex numeric input as interchangeable with decimal input, so this looks like an unintended grammar expansion. Can we keep the previous decimal-only behavior here, or explicitly document and test the new syntax if it’s intentional?

Before #11884 we used strtod, but fastfloat doesn't support hex literals, so in fact it's a break change introduced since that.
Now we bring it back.

SORT BY now fails on valid tiny finite doubles near underflow, while old code passed.

RPUSH s a b
SET w_a 2.2250738585072012e-308
SET w_b 2.2250738585072014e-308
SORT s BY w_*
(error) ERR One or more scores can't be converted into double

good catch.

Maybe we also need to update the README.md file in redis repo.

Maybe we also need to update the README.md file in redis repo.

redis/README.md

Line 225 in 3bcfbbe

sudo apt-get install -y --no-install-recommends ca-certificates wget dpkg-dev gcc g++ libc6-dev libssl-dev make git python3 python3-pip python3-venv python3-dev unzip rsync clang automake autoconf gcc-10 g++-10 libtool

Like this, there is no need to install g++.

no, it's for BUILD_WITH_MODULES

While reviewing the recent fast_float_strtod() changes, I noticed a potential correctness issue in the widened fast path
for 17-19 digit mantissas. This is not about the usual fact that decimal values are often not exactly representable as
binary floating-point values. The concern is that Redis' fast path and its libc strtod() fallback can choose different
adjacent double values for the same valid decimal input.

Issue Found

Redis' current fast_float_strtod() widened fast path can return a different double than libc strtod() for some valid
decimal inputs. The difference is 1 ULP, but it is still observable because Redis uses libc strtod() as the fallback path
and expects the fast path to be bit-exact with it.

Examples:

input: 9007199255094284e-19
fast_float_strtod: 0x3f4d83c94fbbcb8a
libc strtod      : 0x3f4d83c94fbbcb8b
delta            : -1 ULP

input: 2489830482329185244e1
fast_float_strtod: 0x43f59888c51e5b4c
libc strtod      : 0x43f59888c51e5b4b
delta            : +1 ULP

This is not the normal floating-point limitation where a decimal value cannot be represented exactly as binary64. The
expected behavior is still to choose the correctly-rounded nearest representable double. In these cases, the widened fast
path chooses the adjacent double instead.

Reason

The fallback only runs when the fast path rejects an input. These inputs are accepted by the widened fast path, so the
function returns the fast-path result directly and never calls the strtod() fallback.

There are two problematic paths:

• For negative exponents, the widened path computes (mantissa << 64) / divisor using integer division. This truncates the
  remainder, so values near a rounding boundary can be rounded down incorrectly.
• For positive exponents, the widened path splits a __uint128_t product into high/low 64-bit parts, converts each part to
  double, and adds them. This can introduce double rounding and choose the neighboring double.

Because Redis mixes a custom fast path with a runtime libc strtod() fallback, every accepted fast-path input must be bit-
exact with strtod(). The original Clinger fast path had a safe precondition for that. The widened 17-19 digit mantissa path
does not appear to preserve that guarantee for all inputs.

below code is reproduced case.

/*
* Standalone reproducer for Redis fast_float_strtod rounding mismatches.
*
* Build from the repository root:
*
*   cc -std=c11 -O2 -I src tests/helpers/fast_float_strtod_rounding_repro.c -o /tmp/ff_repro
*   /tmp/ff_repro
*
* The program intentionally returns non-zero when it finds a mismatch between
* the current Redis fast_float_strtod implementation and libc strtod().
*/

#include <errno.h>
#include <inttypes.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

/* Pull in the current Redis implementation so the repro follows the real path. */
#include "../../src/fast_float_strtod.c"

/* Minimal allocator stubs used only by fast_float_strtod_fallback(). */
void *zmalloc(size_t size) {
   return malloc(size);
}

void zfree(void *ptr) {
   free(ptr);
}

static uint64_t double_bits(double value) {
   uint64_t bits;
   memcpy(&bits, &value, sizeof(bits));
   return bits;
}

static int check_one(const char *input) {
   char *fast_end = NULL;
   char *libc_end = NULL;
   size_t len = strlen(input);

   errno = 0;
   double fast = fast_float_strtod(input, len, &fast_end);
   int fast_errno = errno;

   errno = 0;
   double libc = strtod(input, &libc_end);
   int libc_errno = errno;

   uint64_t fast_bits = double_bits(fast);
   uint64_t libc_bits = double_bits(libc);
   int64_t ulp_delta = (fast_bits >= libc_bits) ?
       (int64_t)(fast_bits - libc_bits) : -(int64_t)(libc_bits - fast_bits);

   printf("input: %s\n", input);
   printf("  fast_float_strtod: %.17g bits=0x%016" PRIx64 " errno=%d end=%td/%zu\n",
          fast, fast_bits, fast_errno, fast_end - input, len);
   printf("  libc strtod      : %.17g bits=0x%016" PRIx64 " errno=%d end=%td/%zu\n",
          libc, libc_bits, libc_errno, libc_end - input, len);
   printf("  ulp delta        : %" PRId64 "\n\n", ulp_delta);

   return fast_bits != libc_bits || fast_end != input + len || libc_end != input + len;
}

int main(void) {
   const char *cases[] = {
       /* Negative exponent widened path truncates the scaled integer quotient. */
       "9007199255094284e-19",

       /* Positive exponent widened path double-rounds through hi/lo doubles. */
       "2489830482329185244e1",
   };
   int mismatches = 0;

   for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) {
       mismatches += check_one(cases[i]);
   }

   printf("%d mismatch(es) found\n", mismatches);
   return mismatches ? 1 : 0;
}

Test Result

input: 9007199255094284e-19
  fast_float_strtod: 0.00090071992550942834 bits=0x3f4d83c94fbbcb8a errno=0 end=20/20
  libc strtod      : 0.00090071992550942845 bits=0x3f4d83c94fbbcb8b errno=0 end=20/20
  ulp delta        : -1

input: 2489830482329185244e1
  fast_float_strtod: 2.4898304823291855e+19 bits=0x43f59888c51e5b4c errno=0 end=21/21
  libc strtod      : 2.4898304823291851e+19 bits=0x43f59888c51e5b4b errno=0 end=21/21
  ulp delta        : 1

2 mismatch(es) found

While reviewing the recent fast_float_strtod() changes, I noticed a potential correctness issue in the widened fast path for 17-19 digit mantissas. This is not about the usual fact that decimal values are often not exactly representable as binary floating-point values. The concern is that Redis' fast path and its libc strtod() fallback can choose different adjacent double values for the same valid decimal input.

Issue Found

Redis' current fast_float_strtod() widened fast path can return a different double than libc strtod() for some valid decimal inputs. The difference is 1 ULP, but it is still observable because Redis uses libc strtod() as the fallback path and expects the fast path to be bit-exact with it.

Examples:

input: 9007199255094284e-19
fast_float_strtod: 0x3f4d83c94fbbcb8a
libc strtod      : 0x3f4d83c94fbbcb8b
delta            : -1 ULP

input: 2489830482329185244e1
fast_float_strtod: 0x43f59888c51e5b4c
libc strtod      : 0x43f59888c51e5b4b
delta            : +1 ULP

This is not the normal floating-point limitation where a decimal value cannot be represented exactly as binary64. The expected behavior is still to choose the correctly-rounded nearest representable double. In these cases, the widened fast path chooses the adjacent double instead.

Reason

The fallback only runs when the fast path rejects an input. These inputs are accepted by the widened fast path, so the function returns the fast-path result directly and never calls the strtod() fallback.

There are two problematic paths:

• For negative exponents, the widened path computes (mantissa << 64) / divisor using integer division. This truncates the
  remainder, so values near a rounding boundary can be rounded down incorrectly.
• For positive exponents, the widened path splits a __uint128_t product into high/low 64-bit parts, converts each part to
  double, and adds them. This can introduce double rounding and choose the neighboring double.

Because Redis mixes a custom fast path with a runtime libc strtod() fallback, every accepted fast-path input must be bit- exact with strtod(). The original Clinger fast path had a safe precondition for that. The widened 17-19 digit mantissa path does not appear to preserve that guarantee for all inputs.

below code is reproduced case.

/*
* Standalone reproducer for Redis fast_float_strtod rounding mismatches.
*
* Build from the repository root:
*
*   cc -std=c11 -O2 -I src tests/helpers/fast_float_strtod_rounding_repro.c -o /tmp/ff_repro
*   /tmp/ff_repro
*
* The program intentionally returns non-zero when it finds a mismatch between
* the current Redis fast_float_strtod implementation and libc strtod().
*/

#include <errno.h>
#include <inttypes.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

/* Pull in the current Redis implementation so the repro follows the real path. */
#include "../../src/fast_float_strtod.c"

/* Minimal allocator stubs used only by fast_float_strtod_fallback(). */
void *zmalloc(size_t size) {
   return malloc(size);
}

void zfree(void *ptr) {
   free(ptr);
}

static uint64_t double_bits(double value) {
   uint64_t bits;
   memcpy(&bits, &value, sizeof(bits));
   return bits;
}

static int check_one(const char *input) {
   char *fast_end = NULL;
   char *libc_end = NULL;
   size_t len = strlen(input);

   errno = 0;
   double fast = fast_float_strtod(input, len, &fast_end);
   int fast_errno = errno;

   errno = 0;
   double libc = strtod(input, &libc_end);
   int libc_errno = errno;

   uint64_t fast_bits = double_bits(fast);
   uint64_t libc_bits = double_bits(libc);
   int64_t ulp_delta = (fast_bits >= libc_bits) ?
       (int64_t)(fast_bits - libc_bits) : -(int64_t)(libc_bits - fast_bits);

   printf("input: %s\n", input);
   printf("  fast_float_strtod: %.17g bits=0x%016" PRIx64 " errno=%d end=%td/%zu\n",
          fast, fast_bits, fast_errno, fast_end - input, len);
   printf("  libc strtod      : %.17g bits=0x%016" PRIx64 " errno=%d end=%td/%zu\n",
          libc, libc_bits, libc_errno, libc_end - input, len);
   printf("  ulp delta        : %" PRId64 "\n\n", ulp_delta);

   return fast_bits != libc_bits || fast_end != input + len || libc_end != input + len;
}

int main(void) {
   const char *cases[] = {
       /* Negative exponent widened path truncates the scaled integer quotient. */
       "9007199255094284e-19",

       /* Positive exponent widened path double-rounds through hi/lo doubles. */
       "2489830482329185244e1",
   };
   int mismatches = 0;

   for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) {
       mismatches += check_one(cases[i]);
   }

   printf("%d mismatch(es) found\n", mismatches);
   return mismatches ? 1 : 0;
}

Test Result

input: 9007199255094284e-19
  fast_float_strtod: 0.00090071992550942834 bits=0x3f4d83c94fbbcb8a errno=0 end=20/20
  libc strtod      : 0.00090071992550942845 bits=0x3f4d83c94fbbcb8b errno=0 end=20/20
  ulp delta        : -1

input: 2489830482329185244e1
  fast_float_strtod: 2.4898304823291855e+19 bits=0x43f59888c51e5b4c errno=0 end=21/21
  libc strtod      : 2.4898304823291851e+19 bits=0x43f59888c51e5b4b errno=0 end=21/21
  ulp delta        : 1

2 mismatch(es) found

This issue originated from the changes in PR #15061