From 61c85a2b2081dffaf45eb8c6b7754b8d9a80c60d Mon Sep 17 00:00:00 2001 From: filipe oliveira Date: Sun, 4 Dec 2022 08:11:38 +0000 Subject: [PATCH] Speedup GEODIST with fixedpoint_d2string as an optimized version of snprintf %.4f (#11552) GEODIST used snprintf("%.4f") for the reply using addReplyDoubleDistance, which was slow. This PR optimizes it without breaking compatibility by following the approach of ll2string with some changes to match the use case of distance and precision. I.e. we multiply it by 10000 format it as an integer, and then add a decimal point. This can achieve about 35% increase in the achievable ops/sec. Co-authored-by: Oran Agra --- src/geo.c | 2 +- src/util.c | 163 +++++++++++++++++++++++++++++++++++++++++++++ src/util.h | 1 + tests/unit/geo.tcl | 2 + 4 files changed, 167 insertions(+), 1 deletion(-) diff --git a/src/geo.c b/src/geo.c index f4cd44b91..a1781a5b0 100644 --- a/src/geo.c +++ b/src/geo.c @@ -211,7 +211,7 @@ int extractBoxOrReply(client *c, robj **argv, double *conversion, * the kilometer. */ void addReplyDoubleDistance(client *c, double d) { char dbuf[128]; - int dlen = snprintf(dbuf, sizeof(dbuf), "%.4f", d); + const int dlen = fixedpoint_d2string(dbuf, sizeof(dbuf), d, 4); addReplyBulkCBuffer(c, dbuf, dlen); } diff --git a/src/util.c b/src/util.c index ca2a8366b..300d061fc 100644 --- a/src/util.c +++ b/src/util.c @@ -627,6 +627,121 @@ int d2string(char *buf, size_t len, double value) { return len; } +/* Convert a double into a string with 'fractional_digits' digits after the dot precision. + * This is an optimized version of snprintf "%.f". + * We convert the double to long and multiply it by 10 ^ to shift + * the decimal places. + * Note that multiply it of input value by 10 ^ can overflow but on the scenario + * that we currently use within redis this that is not possible. + * After we get the long representation we use the logic from ull2string function on this file + * which is based on the following article: + * https://www.facebook.com/notes/facebook-engineering/three-optimization-tips-for-c/10151361643253920 + * + * Input values: + * char: the buffer to store the string representation + * dstlen: the buffer length + * dvalue: the input double + * fractional_digits: the number of fractional digits after the dot precision. between 1 and 17 + * + * Return values: + * Returns the number of characters needed to represent the number. + * If the buffer is not big enough to store the string, 0 is returned. + */ +int fixedpoint_d2string(char *dst, size_t dstlen, double dvalue, int fractional_digits) { + if (fractional_digits < 1 || fractional_digits > 17) + goto err; + /* min size of 2 ( due to 0. ) + n fractional_digitits + \0 */ + if ((int)dstlen < (fractional_digits+3)) + goto err; + if (dvalue == 0) { + dst[0] = '0'; + dst[1] = '.'; + for (int i = 0; i < fractional_digits; i++) { + dst[2 + i] = '0'; + } + dst[fractional_digits+2] = '\0'; + return fractional_digits + 2; + } + /* scale and round */ + static double powers_of_ten[] = {1.0, 10.0, 100.0, 1000.0, 10000.0, 100000.0, 1000000.0, + 10000000.0, 100000000.0, 1000000000.0, 10000000000.0, 100000000000.0, 1000000000000.0, + 10000000000000.0, 100000000000000.0, 1000000000000000.0, 10000000000000000.0, + 100000000000000000.0 }; + long long svalue = llrint(dvalue * powers_of_ten[fractional_digits]); + unsigned long long value; + /* write sign */ + int negative = 0; + if (svalue < 0) { + if (svalue != LLONG_MIN) { + value = -svalue; + } else { + value = ((unsigned long long) LLONG_MAX)+1; + } + if (dstlen < 2) + goto err; + negative = 1; + dst[0] = '-'; + dst++; + dstlen--; + } else { + value = svalue; + } + + static const char digitsd[201] = + "0001020304050607080910111213141516171819" + "2021222324252627282930313233343536373839" + "4041424344454647484950515253545556575859" + "6061626364656667686970717273747576777879" + "8081828384858687888990919293949596979899"; + + /* Check length. */ + uint32_t ndigits = digits10(value); + if (ndigits >= dstlen) goto err; + int integer_digits = ndigits - fractional_digits; + /* Fractional only check to avoid representing 0.7750 as .7750. + * This means we need to increment the length and store 0 as the first character. + */ + if (integer_digits < 1) { + dst[0] = '0'; + integer_digits = 1; + } + dst[integer_digits] = '.'; + int size = integer_digits + 1 + fractional_digits; + /* fill with 0 if required until fractional_digits */ + for (int i = integer_digits + 1; i < fractional_digits; i++) { + dst[i] = '0'; + } + int next = size - 1; + while (value >= 100) { + int const i = (value % 100) * 2; + value /= 100; + dst[next] = digitsd[i + 1]; + dst[next - 1] = digitsd[i]; + next -= 2; + /* dot position */ + if (next == integer_digits) { + next--; + } + } + + /* Handle last 1-2 digits. */ + if (value < 10) { + dst[next] = '0' + (uint32_t) value; + } else { + int i = (uint32_t) value * 2; + dst[next] = digitsd[i + 1]; + dst[next - 1] = digitsd[i]; + } + /* Null term. */ + dst[size] = '\0'; + return size + negative; +err: + /* force add Null termination */ + if (dstlen > 0) + dst[0] = '\0'; + return 0; +} + /* Trims off trailing zeros from a string representing a double. */ int trimDoubleString(char *buf, size_t len) { if (strchr(buf,'.') != NULL) { @@ -1138,6 +1253,53 @@ static void test_ll2string(void) { assert(!strcmp(buf, "9223372036854775807")); } +static void test_fixedpoint_d2string(void) { + char buf[32]; + double v; + int sz; + v = 0.0; + sz = fixedpoint_d2string(buf, sizeof buf, v, 4); + assert(sz == 6); + assert(!strcmp(buf, "0.0000")); + sz = fixedpoint_d2string(buf, sizeof buf, v, 1); + assert(sz == 3); + assert(!strcmp(buf, "0.0")); + v = 0.01; + sz = fixedpoint_d2string(buf, sizeof buf, v, 4); + assert(sz == 6); + assert(!strcmp(buf, "0.0100")); + sz = fixedpoint_d2string(buf, sizeof buf, v, 1); + assert(sz == 3); + assert(!strcmp(buf, "0.0")); + v = -0.01; + sz = fixedpoint_d2string(buf, sizeof buf, v, 4); + assert(sz == 7); + assert(!strcmp(buf, "-0.0100")); + v = -0.1; + sz = fixedpoint_d2string(buf, sizeof buf, v, 1); + assert(sz == 4); + assert(!strcmp(buf, "-0.1")); + v = 0.1; + sz = fixedpoint_d2string(buf, sizeof buf, v, 1); + assert(sz == 3); + assert(!strcmp(buf, "0.1")); + v = 0.01; + sz = fixedpoint_d2string(buf, sizeof buf, v, 17); + assert(sz == 19); + assert(!strcmp(buf, "0.01000000000000000")); + v = 10.01; + sz = fixedpoint_d2string(buf, sizeof buf, v, 4); + assert(sz == 7); + assert(!strcmp(buf, "10.0100")); + /* negative tests */ + sz = fixedpoint_d2string(buf, sizeof buf, v, 18); + assert(sz == 0); + sz = fixedpoint_d2string(buf, sizeof buf, v, 0); + assert(sz == 0); + sz = fixedpoint_d2string(buf, 1, v, 1); + assert(sz == 0); +} + #define UNUSED(x) (void)(x) int utilTest(int argc, char **argv, int flags) { UNUSED(argc); @@ -1147,6 +1309,7 @@ int utilTest(int argc, char **argv, int flags) { test_string2ll(); test_string2l(); test_ll2string(); + test_fixedpoint_d2string(); return 0; } #endif diff --git a/src/util.h b/src/util.h index 4ff8a88e6..09d80e465 100644 --- a/src/util.h +++ b/src/util.h @@ -74,6 +74,7 @@ int string2ld(const char *s, size_t slen, long double *dp); int string2d(const char *s, size_t slen, double *dp); int trimDoubleString(char *buf, size_t len); int d2string(char *buf, size_t len, double value); +int fixedpoint_d2string(char *dst, size_t dstlen, double dvalue, int fractional_digits); int ld2string(char *buf, size_t len, long double value, ld2string_mode mode); int double2ll(double d, long long *out); int yesnotoi(char *s); diff --git a/tests/unit/geo.tcl b/tests/unit/geo.tcl index 58535bef6..a6a6cbcb6 100644 --- a/tests/unit/geo.tcl +++ b/tests/unit/geo.tcl @@ -405,6 +405,8 @@ start_server {tags {"geo"}} { assert {$m > 166274 && $m < 166275} set km [r geodist points Palermo Catania km] assert {$km > 166.2 && $km < 166.3} + set dist [r geodist points Palermo Palermo] + assert {$dist eq 0.0000} } test {GEODIST missing elements} {