Skip to content

perf: widen fast_float_strtod fast path to 17-19 digit mantissas#15061

Merged
sundb merged 5 commits into
redis:unstablefrom
filipecosta90:zzlstrtod-c-widen-fastpath-clean
Apr 20, 2026
Merged

perf: widen fast_float_strtod fast path to 17-19 digit mantissas#15061
sundb merged 5 commits into
redis:unstablefrom
filipecosta90:zzlstrtod-c-widen-fastpath-clean

Conversation

@fcostaoliveira

@fcostaoliveira fcostaoliveira commented Apr 16, 2026

Copy link
Copy Markdown
Collaborator

Motivation

Bisect (2026-04-16) on memtier_benchmark-1Mkeys-load-zset-listpack-with-100-elements-double-score (x86-aws-m7i.metal-24xl / arm-aws-m8g.metal-24xl, n=3 datapoints per commit) localized a regression on this benchmark to the commit that replaced the C++ fast_float library with the pure-C implementation:

Commit x86 ops/sec ARM ops/sec Δ vs 8.6
a176d12 8.6 branch (8.6.2) 5,633 6,502 baseline
3cd4642 right before the rewrite 5,617 (pending) −0.3% / —
2f1a8b2 right after the rewrite 5,220 4,736 −7.3% / −27.2%
3bcfbbe current unstable 5,310 4,721 −5.7% / −27.4%

All other commits between 8.6 and 3cd4642 are flat vs 8.6 — the single commit that rewrites fast_float_strtod is 100% of the regression on this workload.

Root cause

Roughly 50% of random double scores generated by the ZADD listpack workload have 17-19 significant digits, which exceed MAX_MANTISSA_FAST_PATH (2^53). These inputs fall through to the strtod() fallback:

char static_buf[128];
memcpy(buf, nptr, len);           /* memcpy back! */
buf[len] = '\0';                   /* null-term */
double result = strtod(buf, ...);  /* glibc strtod — ~10× slower on ARM */

The original C++ fast_float library handled the same 17-19 digit inputs with Eisel-Lemire / bigint arithmetic without falling back to strtod(). That is what the pure-C replacement lost.

Fix

Compute mantissa * 10^exponent in 128-bit integer arithmetic using __uint128_t, then convert to double with a single IEEE round-to-nearest-even cast. Supported for |exp| in [0, 19] where 10^|exp| fits in uint64; cases outside that range (or otherwise outside the fast path's preconditions) still fall through to strtod().

Code delta:

  • ~50 LOC in src/fast_float_strtod.c (widened path — no new deps, pure C, no libstdc++ reintroduced)
  • ~25 LOC of unit tests in the existing fastFloatTest
  • ~30 LOC of TCL integration test in tests/unit/type/zset.tcl

Measured perf

1Mkeys-load-zset-listpack-with-100-elements-double-score, n=3 per branch per runner:

Platform 8.6 unstable (pre-PR) After this PR Δ vs 8.6 Δ vs unstable
x86-aws-m7i.metal-24xl 5,633 5,310 9,788 +73.8% +84.3%
arm-aws-m8g.metal-24xl 6,502 4,721 8,724 +34.2% +84.8%

Both arches recover the regression and then some. ARM goes from −27.4% (worst regression in the bisect) to +34.2% vs 8.6.

Correctness

Bit-exact with strtod() on every tested case.

Unit tests (./redis-server test fastfloat): 98 passed, 0 failed. 21 new cases exercising:

  • 17-19 significant digit mantissas with negative exponents (the hot ZADD case)
  • Mantissa exactly at 2^53 boundary + ties-to-even rounding
  • 19-digit integer mantissas
  • Positive exponents through __uint128_t product
  • Exponent boundaries ±19 and cases that fall through to strtod

TCL integration tests (./runtest --single unit/type/zset):

  • Pre-existing ZSCORE after a DEBUG RELOAD test exercises random-double round-trip and passes on both listpack and skiplist encodings.
  • New ZSCORE 17-19 significant digit mantissas (widened fast path) test explicitly asserts bit-exact ZADD → DEBUG RELOAD → ZSCORE round-trip on a curated set of 17-19 digit scores that hit the new code path.

References

The fast-path concept is Clinger's, the pure-C impl being extended is @antirez / @sundb / @mpozniak95 / @moticless's, the broader algorithmic context is fast_float (Lemire & Magalhães). None of those directly cover the narrow "widen the Clinger fast path via __uint128_t one-shot" trick this patch uses — that was written for this patch.

  • Clinger, "How to Read Floating Point Numbers Accurately" (PLDI 1990) — the existing pure-C fast path is a direct application of Clinger; this patch only widens its precondition from mantissa ≤ 2^53 to mantissa ≤ 2^64.
  • Pure-C fast_float_strtod.c being extended: Replace fast_float C++ library with pure C implementation #14661 by @antirez, @sundb, @mpozniak95, @moticless. The existing Clinger fast path (mantissa ≤ 2^53) is unchanged; this patch adds a parallel branch for mantissa > 2^53.
  • Related reading: Lemire & Magalhães, "Number Parsing at a Gigabyte per Second" (2021). A full port via the Eisel-Lemire algorithm (see kolemannix/ffc.h, ~3.5k LOC) achieves correct rounding over the full exponent range. This patch takes the narrower path that only needs ~50 LOC to cover the Redis use case (|exp| ≤ 19).

The specific __uint128_t-based "mantissa × 10^e" / "mantissa ÷ 10^|e|" with single-rounding conversion was written for this PR and is not a direct port from any cited work.


Note

Medium Risk
Touches core numeric parsing logic used across Redis, so any rounding/edge-case mistake could affect persisted scores and comparisons; mitigated by tight bounds (|exp|<=19) and added unit/integration tests.

Overview
Improves decimal parsing performance by extending fast_float_strtod.c’s fast path to handle mantissas larger than 2^53 (17–19 significant digits) using __uint128_t multiply/divide for |exponent| <= 19, reducing fallbacks to strtod() while keeping correct rounding.

Adds targeted coverage for this widened path in the fastFloatTest table (boundary/ties-to-even, positive/negative exponents, negatives) and introduces a new ZSCORE integration test in tests/unit/type/zset.tcl that validates bit-exact ZADD/ZSCORE round-trips (including after DEBUG RELOAD) for representative 17–19 digit scores.

Reviewed by Cursor Bugbot for commit e1e7faf. Bugbot is set up for automated code reviews on this repo. Configure here.

@augmentcode

augmentcode Bot commented Apr 16, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: Extends fast_float_strtod’s fast path to handle 17–19 digit mantissas (up to 64-bit) without falling back to libc strtod(), targeting ZSET score parsing regressions.

Key changes:

  • Adds a widened branch that uses __uint128_t integer arithmetic to compute mantissa × 10^exp (or scaled division for negative exponents) for |exp| ≤ 19
  • Keeps the existing Clinger fast path for mantissa ≤ 2^53 and preserves fallback behavior for out-of-range/complex cases
  • Adds unit tests covering widened-path rounding boundaries and exponent limits
  • Adds a zset integration test validating ZADD → DEBUG RELOAD → ZSCORE round-trips for representative wide-mantissa scores

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. 3 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread src/fast_float_strtod.c
if (exponent >= 0) {
/* (mantissa * 10^e) fits in 128 bits. Convert exactly: the
* single (double) cast from __uint128_t rounds to nearest. */
__uint128_t prod = (__uint128_t)mantissa * (uint64_t)powers_of_ten[exponent];

@augmentcode augmentcode Bot Apr 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

src/fast_float_strtod.c:288: The comment says the __uint128_t product is converted with a “single (double) cast”, but the implementation decomposes into hi/lo and does multiple casts plus an addition. Consider clarifying the rationale/proof that this decomposition is still guaranteed to match strtod() across the widened-fast-path input space.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Comment thread src/fast_float_strtod.c
* 16-19 significant digit case. */
uint64_t divisor = (uint64_t)powers_of_ten[-exponent];
__uint128_t scaled = (__uint128_t)mantissa << 64;
__uint128_t q = scaled / divisor;

@augmentcode augmentcode Bot Apr 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

src/fast_float_strtod.c:304: This negative-exponent widened path uses truncated integer division (scaled / divisor) without looking at the remainder, while the comment claims bit-exact IEEE rounding “by construction”. For values close to a rounding midpoint this could bias results downward unless there’s a proven error bound; consider documenting that bound or adding a tie-case test.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Comment thread src/fast_float_strtod.c

/* 17-19 significant digit mantissas — negative exponent (scores in [0,1)) */
{"0.49606648747577575", 0.49606648747577575}, /* 17 sig digits, ZADD hot case */
{"0.8731899671198792", 0.8731899671198792}, /* 16 sig digits */

@augmentcode augmentcode Bot Apr 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

src/fast_float_strtod.c:489: This block is labeled as covering the widened mantissa > 2^53 / __uint128_t path, but inputs like 0.8731899671198792 (and 0.999999999999999) have mantissas ≤ 2^53 so they’ll likely exercise the original Clinger branch instead. That reduces coverage of the new path and may hide regressions.

Severity: low

Other Locations
  • src/fast_float_strtod.c:493
  • tests/unit/type/zset.tcl:1775

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Comment thread src/fast_float_strtod.c
uint64_t hi = (uint64_t)(q >> 64);
uint64_t lo = (uint64_t)q;
value = ((double)hi * 18446744073709551616.0 + (double)lo)
* 5.421010862427522170037e-20; /* 2^-64 */

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hi-lo split double rounding produces 1-ULP incorrect results

Medium Severity

The hi-lo split (double)hi * 2^64 + (double)lo does not always produce a correctly-rounded double for the 128-bit integer q. When hi exceeds 2^53 and (double)hi rounds down (ties-to-even), the lo component is too small relative to the double's ULP at that scale to compensate, so it gets absorbed by the IEEE addition. This causes 1-ULP errors versus strtod. For example, input "9007199254740993.5" returns 9007199254740992.0 instead of the correct 9007199254740994.0. The pattern triggers whenever hi is an odd integer just above 2^53 (causing a round-down) and lo > 0. The same issue affects both the positive and negative exponent widened paths.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 82e82a5. Configure here.

Recovers the regression on float-parsing-heavy sorted-set listpack
workloads introduced when the C++ fast_float library was replaced by
the pure-C implementation. Bisect on
`memtier_benchmark-1Mkeys-load-zset-listpack-with-100-elements-double-score`
(x86-aws-m7i.metal-24xl / arm-aws-m8g.metal-24xl, n=3 datapoints each):

| Commit                               | x86 ops/sec | ARM ops/sec |
|--------------------------------------|------------:|------------:|
| 8.6 branch (a176d12)                 |       5,633 |       6,502 |
| Right before the rewrite             |       5,617 |         TBD |
| Current unstable (with the rewrite)  |       5,310 |       4,721 |
| **After this patch (x86 measured)**  |   **9,788** |         TBD |

x86 result: +73.8% vs 8.6, +84.3% vs unstable. ARM measurement pending.

## Root cause

~50% of random double scores have 17-19 significant digits, which
exceed MAX_MANTISSA_FAST_PATH (2^53). These fell through to the
strtod() fallback (memcpy + null-term + glibc strtod), which on ARM
is significantly slower than the fast path and is essentially the
entire regression vs 8.6.

## Fix

Compute (mantissa * 10^exp) in 128-bit integer arithmetic via
`__uint128_t` and convert to double with a single IEEE round-to-
nearest-even cast. Supported for |exp| in [0, 19] where 10^|exp|
fits in uint64. Cases outside that range (or truly corrupt inputs)
still fall through to strtod().

## Correctness

Bit-exact with strtod on every tested case:

- `src/fast_float_strtod.c` unit tests: 98 passed (77 original + 21
  new), 0 failed. New cases exercise positive/negative exponents,
  mantissa exactly at 2^53 boundary, ties-to-even rounding, 19-digit
  mantissas, cases that fall back to strtod.
- `tests/unit/type/zset.tcl` integration tests: round-trip ZADD →
  DEBUG RELOAD → ZSCORE on listpack + skiplist encodings with 17-19
  significant digit scores.

## Code delta

~50 LOC in fast_float_strtod.c, ~25 LOC of unit tests, ~30 LOC of
TCL integration test. No new dependencies; pure C; no libstdc++
reintroduced.

## References

The fast-path concept is Clinger's, the pure-C impl being extended
is antirez/sundb/mpozniak95/moticless's, the broader algorithmic
context is fast_float (Lemire & Magalhães). None of those directly
cover the narrow 'widen the Clinger fast path via __uint128_t
one-shot' trick this patch uses — that was written for this patch.

- Clinger, 'How to Read Floating Point Numbers Accurately' (PLDI
  1990) — the existing pure-C fast path is a direct application of
  Clinger; this patch only widens its precondition.
- Pure-C fast_float_strtod.c being extended: redis#14661 by
  @antirez, @sundb, @mpozniak95, @moticless. The existing Clinger
  fast path (mantissa <= 2^53) is unchanged.
- Related reading: Lemire & Magalhães, 'Number Parsing at a Gigabyte
  per Second' (2021). A full port via the Eisel-Lemire algorithm
  (see github.com/kolemannix/ffc.h, ~3.5k LOC) achieves correct
  rounding over the full exponent range. This patch takes the
  narrower path that only needs ~50 LOC to cover the Redis use case.
@fcostaoliveira fcostaoliveira force-pushed the zzlstrtod-c-widen-fastpath-clean branch from 82e82a5 to 7de270d Compare April 17, 2026 09:43
Comment thread src/fast_float_strtod.c
Co-authored-by: debing.sun <debing.sun@redis.com>
@fcostaoliveira fcostaoliveira requested a review from sundb April 17, 2026 10:33
Comment thread src/fast_float_strtod.c Outdated
@sundb

sundb commented Apr 19, 2026

Copy link
Copy Markdown
Collaborator

Please also make vecSetFreeMethod() a static inline function in the header and, while at it, consider making vecData() and vecSize() macros inline as well.

@moticless post in the wrong place.

Comment thread tests/unit/type/zset.tcl
Co-authored-by: debing.sun <debing.sun@redis.com>
@fcostaoliveira fcostaoliveira requested a review from sundb April 20, 2026 08:04

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

Reviewed by Cursor Bugbot for commit d0ad649. Configure here.

Comment thread tests/unit/type/zset.tcl
Without `set i 0` / `incr i`, the `foreach` loop reuses a stale `$i`
from the preceding ZSCORE test (equal to `$elements`), so every ZADD
writes the same member and the test no longer validates round-trip
correctness for multiple 17-19 significant-digit mantissas. After
`debug reload` only the last score is stored, and the post-reload
assertions would not detect a regression in any mantissa other than
the last one.
@sundb sundb merged commit 0fa78fd into redis:unstable Apr 20, 2026
18 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in Redis 8.8 Apr 20, 2026
@vitahlin

Copy link
Copy Markdown
Contributor

Regarding the comment: #14661 (comment)

I’ve tested this locally using the Redis unit test suite and found that the PR might fail on specific rounding-boundary cases in the widened fast path.

To reproduce, you can add these cases to fast_float_strtod.c at line 527:

 /* Widened-fast-path rounding-boundary cases. */
{"9007199255094284e-19", 9007199255094284e-19},
{"2489830482329185244e1", 2489830482329185244e1},

Then run the test using:

$ make -C src REDIS_CFLAGS=-DREDIS_TEST redis-server
$ ./src/redis-server test fastfloat                 
CleanShot 2026-04-26 at 01 59 17@2x

YaacovHazan pushed a commit that referenced this pull request Apr 28, 2026
)

## Root cause

Roughly 50% of random double scores generated by the ZADD listpack
workload have 17-19 significant digits, which exceed
`MAX_MANTISSA_FAST_PATH` (`2^53`). These inputs fall through to the
`strtod()` fallback:

```c
char static_buf[128];
memcpy(buf, nptr, len);           /* memcpy back! */
buf[len] = '\0';                   /* null-term */
double result = strtod(buf, ...);  /* glibc strtod — ~10× slower on ARM */
```

The original C++ `fast_float` library handled the same 17-19 digit
inputs with Eisel-Lemire / bigint arithmetic without falling back to
`strtod()`. That is what the pure-C replacement lost.

## Fix

Compute `mantissa * 10^exponent` in 128-bit integer arithmetic using
`__uint128_t`, then convert to double with a single IEEE
round-to-nearest-even cast. Supported for `|exp| in [0, 19]` where
`10^|exp|` fits in `uint64`; cases outside that range (or otherwise
outside the fast path's preconditions) still fall through to `strtod()`.

---------

Co-authored-by: debing.sun <debing.sun@redis.com>
sundb pushed a commit that referenced this pull request May 13, 2026
…15111)

## Summary

Fixes a ±1 ULP rounding mismatch between `fast_float_strtod()` and libc
`strtod()` in the widened (mantissa > 2^53) fast path introduced by
#15061. Reported by @vitahlin in
#14661 (comment) with
two minimal reproducers:

```
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
```

Redis treats `strtod()` as the fallback for `fast_float_strtod()`, so
every fast-path-accepted input is contractually expected to be bit-exact
with `strtod()`. The two cases above are accepted by the widened branch
but produce a different IEEE-754 representation, breaking the contract.

## Root cause

The widened branch added in #15061 used a homebrew shortcut to convert a
128-bit integer product to a double:

```c
value = (double)hi * 18446744073709551616.0 + (double)lo;
```

This is **not** a single-rounding operation — `(double)hi` rounds when
`hi > 2^53`, and the subsequent `+ (double)lo` rounds again. For inputs
near the round-half-to-even boundary, the two roundings can compose into
the wrong direction.

The negative-exponent branch is doubly affected: integer division
`scaled / divisor` truncates the remainder before the conversion, so
even a hypothetical correct `hi*2^64+lo` step would round down on inputs
that should round up.

## Fix

Replace the homebrew widened branch with the **Eisel-Lemire** algorithm
from upstream `fast_float`. This is the same algorithm that
`fast_float`'s own widened path uses; bit-exact-with-strtod for inputs
with ≤19-digit mantissa is proved in:

> Noble Mushtak and Daniel Lemire, "Fast Number Parsing Without
Fallback".

Pieces ported (MIT-licensed, from
`fast_float/include/fast_float/fast_table.h`, `decimal_to_binary.h`,
`float_common.h`):

- 128-bit precomputed extended-precision powers of five (`5^-342 ...
5^308`, 651 entries) — pure data.
- `compute_product_approximation` — 128-bit multiply with high-half
rounding-boundary fixup.
- `compute_float` — the main algorithm; returns `(mantissa, power2)`
ready to be packed.
- `am_to_double` — IEEE-754 binary64 bit-pack.
- `__builtin_clzll` and `__uint128_t` wrappers (with a 32-bit fallback
for portability).

Clinger's strict fast path (`mantissa ≤ 2^53` and `|exp| ≤ 22`) is
**kept** unchanged — it's a single double multiply/divide and is faster
than Eisel-Lemire on its domain. Only the buggy widened branch is
replaced.

The port stays minimal:
- **double-only** (no `float`, no `long double`)
- No bigint slow path. The rare "indeterminate" inputs that upstream
resolves with `digit_comp` are unreachable from `parse_number_string`'s
≤19-digit mantissa per the Mushtak-Lemire proof, but a defensive
`am.power2 < 0` check is preserved that falls back to libc `strtod()` if
any future caller widens the input domain.

## Why not just revert #15061?

Considered. Reverting restores correctness at the cost of the +73-84 %
zset listpack-load wins #15061 measured on 17-19 digit double scores.
Eisel-Lemire is *the* algorithm that gives both correctness *and* the
wider mantissa range — preserving #15061's wins while fixing the
rounding regression.

A "tightened admission filter" (only accept widened-path inputs where
the conversion happens to be single-rounding) was also considered. The
math shows the filter conditions are essentially unsatisfied for typical
inputs (`lo == 0` requires the 128-bit product be divisible by 2^64;
only ~1 in 10^13 random inputs qualify), making it equivalent to a
revert with extra dead code. Eisel-Lemire is the only widened-path
solution that preserves perf on the typical case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants