mirror of
https://github.com/redis/redis.git
synced 2026-10-28 08:25:04 +08:00
deps/fpconv: remove every integer division from Grisu digit generation (#15787)
`generate_digits()` divides `part1` by a divisor loaded from the static `tens[]` table. Because the table is mutable, the compiler can't assume its contents and emits a real 64-bit `divq` for every fractional double formatted into a RESP reply. ## Change 1. `deps/fpconv/fpconv_dtoa.c`: `static uint64_t tens[]` → `static const uint64_t tens[]` (plus the two pointers into it). 2. `deps/fpconv/Makefile`: `OPT= -Os` → `OPT= -O3`. `fpconv_dtoa.o` goes from 5 `div` instructions to 0. Output is byte-for-byte identical. ## Why both changes are needed 1. The loop must be unrolled so each divisor is a known constant. GCC only does that at `-O3`. 2. The compiler must be able to prove the divisor values. With a mutable table, plain `-O3` is a regression (10 divides). 3. The compiler must choose to strength-reduce. At `-Os` it declines, so `const` alone changes nothing. `deps/*/Makefile` files carry their own `OPT` and `src/Makefile`'s `-O3` doesn't propagate, so `deps/fpconv` has been building at `-Os`. `div` instructions in `fpconv_dtoa.o` (gcc 11.4 and 13.3): | build | `-Os` | `-O2` | `-O3` | |---|---|---|---| | before | 5 | 1 | 10 | | this PR | 5 | 1 | **0** | clang 18/19 already reach 0 at `-O3` without `const`, so there it is documentation only. ## Results `redis-benchmarks-specification`, Xeon 6972P, server and client on separate NUMA nodes, arms interleaved, 5 reps per cell. | test | before → after (ops/sec) | Δ | |---|---|---| | `zset-1K zrange-all-elements` | 6,797 → 9,616 | **+41.5%** | | `zset-100 zrange-all-elements` | 42,181 → 48,555 | **+15.1%** | | `zrangebyscore long-scores` (integer scores, control) | 69,321 → 69,420 | +0.14% | The control takes the `double2ll()` fast path and never reaches Grisu, so the gain comes from the float-formatting path. With `io-threads 16` the numbers are similar (+46.3% / +22.2% / +0.02%). Every RESP double reply benefits: `ZSCORE`, `ZINCRBY`, `ZRANGE ... WITHSCORES`, `GEODIST`, `INCRBYFLOAT`, etc. Integer values are unaffected. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: fcostaoliveira <filipe@redis.com>
This commit is contained in:
2 files changed
+27
-23
No files matched your search
Vendored
+1
-1
@@ -1,6 +1,6 @@
|
||||
STD=
|
||||
WARN= -Wall
|
||||
OPT= -Os
|
||||
OPT= -O3
|
||||
|
||||
R_CFLAGS= $(STD) $(WARN) $(OPT) $(DEBUG) $(CFLAGS)
|
||||
R_LDFLAGS= $(LDFLAGS)
|
||||
|
||||
Vendored
+26
-22
@@ -51,26 +51,30 @@
|
||||
#define absv(n) ((n) < 0 ? -(n) : (n))
|
||||
#define minv(a, b) ((a) < (b) ? (a) : (b))
|
||||
|
||||
static uint64_t tens[] = { 10000000000000000000U,
|
||||
1000000000000000000U,
|
||||
100000000000000000U,
|
||||
10000000000000000U,
|
||||
1000000000000000U,
|
||||
100000000000000U,
|
||||
10000000000000U,
|
||||
1000000000000U,
|
||||
100000000000U,
|
||||
10000000000U,
|
||||
1000000000U,
|
||||
100000000U,
|
||||
10000000U,
|
||||
1000000U,
|
||||
100000U,
|
||||
10000U,
|
||||
1000U,
|
||||
100U,
|
||||
10U,
|
||||
1U };
|
||||
/* Must stay const: generate_digits() divides part1 by tens[i], and the compiler
|
||||
* can only replace those divisions with multiply-shifts if it may assume the
|
||||
* table's contents. Dropping const puts a real 64-bit divq back on the hot
|
||||
* double->string path. */
|
||||
static const uint64_t tens[] = { 10000000000000000000U,
|
||||
1000000000000000000U,
|
||||
100000000000000000U,
|
||||
10000000000000000U,
|
||||
1000000000000000U,
|
||||
100000000000000U,
|
||||
10000000000000U,
|
||||
1000000000000U,
|
||||
100000000000U,
|
||||
10000000000U,
|
||||
1000000000U,
|
||||
100000000U,
|
||||
10000000U,
|
||||
1000000U,
|
||||
100000U,
|
||||
10000U,
|
||||
1000U,
|
||||
100U,
|
||||
10U,
|
||||
1U };
|
||||
|
||||
static inline uint64_t get_dbits(double d) {
|
||||
union
|
||||
@@ -176,7 +180,7 @@ static int generate_digits(Fp *fp, Fp *upper, Fp *lower, char *digits, int *K) {
|
||||
uint64_t part2 = upper->frac & (one.frac - 1);
|
||||
|
||||
int idx = 0, kappa = 10;
|
||||
uint64_t *divp;
|
||||
const uint64_t *divp;
|
||||
/* 1000000000 */
|
||||
for (divp = tens + 10; kappa > 0; divp++) {
|
||||
uint64_t div = *divp;
|
||||
@@ -199,7 +203,7 @@ static int generate_digits(Fp *fp, Fp *upper, Fp *lower, char *digits, int *K) {
|
||||
}
|
||||
|
||||
/* 10 */
|
||||
uint64_t *unit = tens + 18;
|
||||
const uint64_t *unit = tens + 18;
|
||||
|
||||
while (true) {
|
||||
part2 *= 10;
|
||||
|
||||
Reference in new issue
Block a user