From f3c6f9c2f44a7baf14de0f98b2b89c5c6c2781ba Mon Sep 17 00:00:00 2001 From: filipe oliveira Date: Mon, 6 Feb 2023 16:26:40 +0000 Subject: [PATCH] Optimize ZRANGE replies WITHSCORES in case of integer scores (#11779) If we have integer scores on the sorted set we're not using the fastest way to reply by calling `d2string` which uses `double2ll` and `ll2string` when it can, instead of `fpconv_dtoa`. This results by some 50% performance improvement in certain cases of integer scores for both RESP2 and RESP3, and no apparent impact on double scores. Co-authored-by: Oran Agra --- src/networking.c | 73 ++++++++++++++------------------------- src/util.c | 5 +++ tests/integration/rdb.tcl | 2 +- 3 files changed, 32 insertions(+), 48 deletions(-) diff --git a/src/networking.c b/src/networking.c index 9eb557812..8f802532d 100644 --- a/src/networking.c +++ b/src/networking.c @@ -835,57 +835,36 @@ void setDeferredPushLen(client *c, void *node, long length) { /* Add a double as a bulk reply */ void addReplyDouble(client *c, double d) { - if (isinf(d)) { - /* Libc in odd systems (Hi Solaris!) will format infinite in a - * different way, so better to handle it in an explicit way. */ - if (c->resp == 2) { - addReplyBulkCString(c, d > 0 ? "inf" : "-inf"); - } else { - addReplyProto(c, d > 0 ? ",inf\r\n" : ",-inf\r\n", - d > 0 ? 6 : 7); - } - } else if (isnan(d)) { - /* Libc in some systems will format nan in a different way, - * like nan, -nan, NAN, nan(char-sequence). - * So we normalize it and create a single nan form in an explicit way. */ - if (c->resp == 2) { - addReplyBulkCString(c, "nan"); - } else { - addReplyProto(c, ",nan\r\n", 6); - } + if (c->resp == 3) { + char dbuf[MAX_D2STRING_CHARS+3]; + dbuf[0] = ','; + const int dlen = d2string(dbuf+1,sizeof(dbuf)-1,d); + dbuf[dlen+1] = '\r'; + dbuf[dlen+2] = '\n'; + dbuf[dlen+3] = '\0'; + addReplyProto(c,dbuf,dlen+3); } else { char dbuf[MAX_LONG_DOUBLE_CHARS+32]; - int dlen = 0; - if (c->resp == 2) { - /* In order to prepend the string length before the formatted number, - * but still avoid an extra memcpy of the whole number, we reserve space - * for maximum header `$0000\r\n`, print double, add the resp header in - * front of it, and then send the buffer with the right `start` offset. */ - dlen = fpconv_dtoa(d, dbuf+7); - int digits = digits10(dlen); - int start = 4 - digits; - dbuf[start] = '$'; + /* In order to prepend the string length before the formatted number, + * but still avoid an extra memcpy of the whole number, we reserve space + * for maximum header `$0000\r\n`, print double, add the resp header in + * front of it, and then send the buffer with the right `start` offset. */ + const int dlen = d2string(dbuf+7,sizeof(dbuf)-7,d); + int digits = digits10(dlen); + int start = 4 - digits; + dbuf[start] = '$'; - /* Convert `dlen` to string, putting it's digits after '$' and before the - * formatted double string. */ - for(int i = digits, val = dlen; val && i > 0 ; --i, val /= 10) { - dbuf[start + i] = "0123456789"[val % 10]; - } - - dbuf[5] = '\r'; - dbuf[6] = '\n'; - dbuf[dlen+7] = '\r'; - dbuf[dlen+8] = '\n'; - dbuf[dlen+9] = '\0'; - addReplyProto(c,dbuf+start,dlen+9-start); - } else { - dbuf[0] = ','; - dlen = fpconv_dtoa(d, dbuf+1); - dbuf[dlen+1] = '\r'; - dbuf[dlen+2] = '\n'; - dbuf[dlen+3] = '\0'; - addReplyProto(c,dbuf,dlen+3); + /* Convert `dlen` to string, putting it's digits after '$' and before the + * formatted double string. */ + for(int i = digits, val = dlen; val && i > 0 ; --i, val /= 10) { + dbuf[start + i] = "0123456789"[val % 10]; } + dbuf[5] = '\r'; + dbuf[6] = '\n'; + dbuf[dlen+7] = '\r'; + dbuf[dlen+8] = '\n'; + dbuf[dlen+9] = '\0'; + addReplyProto(c,dbuf+start,dlen+9-start); } } diff --git a/src/util.c b/src/util.c index 74508a70f..aeeafb915 100644 --- a/src/util.c +++ b/src/util.c @@ -601,8 +601,13 @@ int double2ll(double d, long long *out) { * into a listpack representing a sorted set. */ int d2string(char *buf, size_t len, double value) { if (isnan(value)) { + /* Libc in some systems will format nan in a different way, + * like nan, -nan, NAN, nan(char-sequence). + * So we normalize it and create a single nan form in an explicit way. */ len = snprintf(buf,len,"nan"); } else if (isinf(value)) { + /* Libc in odd systems (Hi Solaris!) will format infinite in a + * different way, so better to handle it in an explicit way. */ if (value < 0) len = snprintf(buf,len,"-inf"); else diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl index d4c6227fc..1479b500f 100644 --- a/tests/integration/rdb.tcl +++ b/tests/integration/rdb.tcl @@ -30,7 +30,7 @@ start_server [list overrides [list "dir" $server_path "dbfilename" "encodings.rd "0","set_zipped_2","set","100000","200000","300000","400000", "0","set_zipped_3","set","1000000000","2000000000","3000000000","4000000000","5000000000","6000000000", "0","string","string","Hello World" -"0","zset","zset","a","1","b","2","c","3","aa","10","bb","20","cc","30","aaa","100","bbb","200","ccc","300","aaaa","1000","cccc","123456789","bbbb","5e+9", +"0","zset","zset","a","1","b","2","c","3","aa","10","bb","20","cc","30","aaa","100","bbb","200","ccc","300","aaaa","1000","cccc","123456789","bbbb","5000000000", "0","zset_zipped","zset","a","1","b","2","c","3", } }