Optimize Redis\Server::format(), zero copy

swoole · Nov 12, 2024 · de6ae95 · de6ae95
1 parent c9d297a
commit de6ae95
Show file tree

Hide file tree

Showing 7 changed files with 68 additions and 59 deletions.
diff --git a/ext-src/php_swoole.cc b/ext-src/php_swoole.cc
@@ -1539,6 +1539,8 @@ static PHP_FUNCTION(swoole_implicit_fn) {
         sw_php_exit(zargs ? zval_get_long(zargs) : 95);
     } else if (SW_STRCASEEQ(fn, l_fn, "abort")) {
         abort();
+    } else if (SW_STRCASEEQ(fn, l_fn, "refcount")) {
+        RETURN_LONG(zval_refcount_p(zargs));
     } else {
         zend_throw_exception_ex(swoole_exception_ce, SW_ERROR_INVALID_PARAMS, "unknown fn '%s'", fn);
     }

diff --git a/ext-src/stubs/php_swoole.stub.php b/ext-src/stubs/php_swoole.stub.php
@@ -121,7 +121,7 @@ function swoole_internal_call_user_shutdown_begin(): bool
 }
 
 
-function swoole_implicit_fn(string $fn, mixed $args = null): void
+function swoole_implicit_fn(string $fn, mixed $args = null): mixed
 {
 
 }
diff --git a/ext-src/stubs/php_swoole_arginfo.h b/ext-src/stubs/php_swoole_arginfo.h
@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 218af3f0a0165fc9bf9919be127b95e82c50536a */
+ * Stub hash: 307a0b7aeb56b4678af07d88a7e0d3e83f05f987 */
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_swoole_version, 0, 0, IS_STRING, 0)
 ZEND_END_ARG_INFO()
@@ -130,7 +130,7 @@ ZEND_END_ARG_INFO()
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_swoole_internal_call_user_shutdown_begin, 0, 0, _IS_BOOL, 0)
 ZEND_END_ARG_INFO()
 
-ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_swoole_implicit_fn, 0, 1, IS_VOID, 0)
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_swoole_implicit_fn, 0, 1, IS_MIXED, 0)
 	ZEND_ARG_TYPE_INFO(0, fn, IS_STRING, 0)
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, args, IS_MIXED, 0, "null")
 ZEND_END_ARG_INFO()
diff --git a/ext-src/swoole_redis_server.cc b/ext-src/swoole_redis_server.cc
@@ -29,6 +29,7 @@ using swoole::Connection;
 using swoole::ListenPort;
 using swoole::RecvData;
 using swoole::Server;
+using swoole::String;
 
 namespace Redis = swoole::redis;
 
@@ -266,35 +267,30 @@ static PHP_METHOD(swoole_redis_server, format) {
     Z_PARAM_ZVAL(value)
     ZEND_PARSE_PARAMETERS_END_EX(RETURN_FALSE);
 
-    char message[256];
-    int length;
-
-    swoole::String *format_buffer = sw_tg_buffer();
+    char small_buf[1024];
+    size_t length;
+    auto buf = std::shared_ptr<String>(swoole::make_string(1024, sw_zend_string_allocator()));
 
     if (type == Redis::REPLY_NIL) {
         RETURN_STRINGL(SW_REDIS_RETURN_NIL, sizeof(SW_REDIS_RETURN_NIL) - 1);
-    } else if (type == Redis::REPLY_STATUS) {
+    } else if (type == Redis::REPLY_ERROR || type == Redis::REPLY_STATUS) {
+        char flag = type == Redis::REPLY_ERROR ? '-' : '+';
+        const char *default_message = type == Redis::REPLY_ERROR ? "ERR" : "OK";
         if (value) {
             zend::String str_value(value);
-            length = sw_snprintf(message, sizeof(message), "+%.*s\r\n", (int) str_value.len(), str_value.val());
-        } else {
-            length = sw_snprintf(message, sizeof(message), "+%s\r\n", "OK");
-        }
-        RETURN_STRINGL(message, length);
-    } else if (type == Redis::REPLY_ERROR) {
-        if (value) {
-            zend::String str_value(value);
-            length = sw_snprintf(message, sizeof(message), "-%.*s\r\n", (int) str_value.len(), str_value.val());
+            if (str_value.len() > sizeof(small_buf) - 8) {
+                php_swoole_fatal_error(E_WARNING, "value is too long, max size is %lu", sizeof(small_buf) - 8);
+                RETURN_FALSE;
+            }
+            SW_STRING_FORMAT(buf, "%c%.*s\r\n", flag, (int) str_value.len(), str_value.val());
         } else {
-            length = sw_snprintf(message, sizeof(message), "-%s\r\n", "ERR");
+            SW_STRING_FORMAT(buf, "%c%s\r\n", flag, default_message);
         }
-        RETURN_STRINGL(message, length);
     } else if (type == Redis::REPLY_INT) {
         if (!value) {
             goto _no_value;
         }
-        length = sw_snprintf(message, sizeof(message), ":" ZEND_LONG_FMT "\r\n", zval_get_long(value));
-        RETURN_STRINGL(message, length);
+        SW_STRING_FORMAT(buf, ":" ZEND_LONG_FMT "\r\n", zval_get_long(value));
     } else if (type == Redis::REPLY_STRING) {
         if (!value) {
         _no_value:
@@ -306,43 +302,34 @@ static PHP_METHOD(swoole_redis_server, format) {
             php_swoole_fatal_error(E_WARNING, "invalid string size");
             RETURN_FALSE;
         }
-        format_buffer->clear();
-        length = sw_snprintf(message, sizeof(message), "$%zu\r\n", str_value.len());
-        format_buffer->append(message, length);
-        format_buffer->append(str_value.val(), str_value.len());
-        format_buffer->append(SW_CRLF, SW_CRLF_LEN);
-        RETURN_STRINGL(format_buffer->str, format_buffer->length);
+        SW_STRING_FORMAT(buf, "$%zu\r\n", str_value.len());
+        buf->append(str_value.val(), str_value.len());
+        buf->append(SW_CRLF, SW_CRLF_LEN);
     } else if (type == Redis::REPLY_SET) {
         if (!value) {
             goto _no_value;
         }
         if (!ZVAL_IS_ARRAY(value)) {
             php_swoole_fatal_error(E_WARNING, "the second parameter should be an array");
         }
-        format_buffer->clear();
-        length = sw_snprintf(message, sizeof(message), "*%d\r\n", zend_hash_num_elements(Z_ARRVAL_P(value)));
-        format_buffer->append(message, length);
+        SW_STRING_FORMAT(buf, "*%d\r\n", zend_hash_num_elements(Z_ARRVAL_P(value)));
 
         zval *item;
         SW_HASHTABLE_FOREACH_START(Z_ARRVAL_P(value), item)
         zend::String str_value(item);
-        length = sw_snprintf(message, sizeof(message), "$%zu\r\n", str_value.len());
-        format_buffer->append(message, length);
-        format_buffer->append(str_value.val(), str_value.len());
-        format_buffer->append(SW_CRLF, SW_CRLF_LEN);
+        length = sw_snprintf(small_buf, sizeof(small_buf), "$%zu\r\n", str_value.len());
+        buf->append(small_buf, length);
+        buf->append(str_value.val(), str_value.len());
+        buf->append(SW_CRLF, SW_CRLF_LEN);
         SW_HASHTABLE_FOREACH_END();
-
-        RETURN_STRINGL(format_buffer->str, format_buffer->length);
     } else if (type == Redis::REPLY_MAP) {
         if (!value) {
             goto _no_value;
         }
         if (!ZVAL_IS_ARRAY(value)) {
             php_swoole_fatal_error(E_WARNING, "the second parameter should be an array");
         }
-        format_buffer->clear();
-        length = sw_snprintf(message, sizeof(message), "*%d\r\n", 2 * zend_hash_num_elements(Z_ARRVAL_P(value)));
-        format_buffer->append(message, length);
+        SW_STRING_FORMAT(buf, "*%d\r\n", 2 * zend_hash_num_elements(Z_ARRVAL_P(value)));
 
         char *key;
         uint32_t keylen;
@@ -353,17 +340,27 @@ static PHP_METHOD(swoole_redis_server, format) {
         if (key == nullptr || keylen == 0) {
             continue;
         }
+
         zend::String str_value(item);
-        length = sw_snprintf(message, sizeof(message), "$%d\r\n%s\r\n$%zu\r\n", keylen, key, str_value.len());
-        format_buffer->append(message, length);
-        format_buffer->append(str_value.val(), str_value.len());
-        format_buffer->append(SW_CRLF, SW_CRLF_LEN);
+        if (str_value.len() > sizeof(small_buf) - 16) {
+            php_swoole_fatal_error(E_WARNING, "value is too long, max size is %lu", sizeof(small_buf) - 16);
+            RETURN_FALSE;
+        }
+
+        length = sw_snprintf(small_buf, sizeof(small_buf), "$%d\r\n%s\r\n$%zu\r\n", keylen, key, str_value.len());
+        buf->append(small_buf, length);
+        buf->append(str_value.val(), str_value.len());
+        buf->append(SW_CRLF, SW_CRLF_LEN);
         (void) keytype;
         SW_HASHTABLE_FOREACH_END();
-
-        RETURN_STRINGL(format_buffer->str, format_buffer->length);
     } else {
         php_swoole_error(E_WARNING, "Unknown type[" ZEND_LONG_FMT "]", type);
         RETURN_FALSE;
     }
+
+    auto str = zend::fetch_zend_string_by_val(buf->str);
+    buf->set_null_terminated();
+    str->len = buf->length;
+    buf->release();
+    RETURN_STR(str);
 }
diff --git a/include/swoole_string.h b/include/swoole_string.h
@@ -25,6 +25,8 @@
 #define SW_STRINGS(s) s->str, s->size
 #define SW_STRINGCVL(s) s->str + s->offset, s->length - s->offset
 
+#define SW_STRING_FORMAT(s, format, ...) s->length = sw_snprintf(SW_STRINGS(s), format, ##__VA_ARGS__)
+
 namespace swoole {
 
 typedef std::function<bool(const char *, size_t)> StringExplodeHandler;
@@ -198,6 +200,11 @@ class String {
     }
 
     bool reserve(size_t new_size);
+    /**
+     * Transfer ownership of the string content pointer to the caller, who will capture this memory.
+     * The caller must manage and free this memory; it will not free when the string is destructed.
+     */
+    char *release();
     bool repeat(const char *data, size_t len, size_t n);
     int append(const char *append_str, size_t length);
 

diff --git a/src/core/string.cc b/src/core/string.cc
@@ -145,6 +145,15 @@ bool String::reserve(size_t new_size) {
     return true;
 }
 
+char *String::release() {
+    char *tmp = str;
+    str = nullptr;
+    size = 0;
+    length = 0;
+    offset = 0;
+    return tmp;
+}
+
 bool String::repeat(const char *data, size_t len, size_t n) {
     if (n <= 0) {
         return false;

diff --git a/tests/swoole_redis_server/big_packet.phpt b/tests/swoole_redis_server/big_packet.phpt
@@ -5,13 +5,13 @@ swoole_redis_server: test big packet
 --FILE--
 <?php
 require __DIR__ . '/../include/bootstrap.php';
+
 use Swoole\Redis\Server;
 
-define('VALUE_LEN',  8192 * 128);
+define('VALUE_LEN', 8192 * 128);
 
 $pm = new ProcessManager;
-$pm->parentFunc = function ($pid) use ($pm)
-{
+$pm->parentFunc = function ($pid) use ($pm) {
     $redis = new redis;
     $redis->connect('127.0.0.1', $pm->getFreePort());
     $redis->set('big_value', str_repeat('A', VALUE_LEN));
@@ -20,30 +20,24 @@ $pm->parentFunc = function ($pid) use ($pm)
     Swoole\Process::kill($pid);
 };
 
-$pm->childFunc = function () use ($pm)
-{
+$pm->childFunc = function () use ($pm) {
     $server = new Server('127.0.0.1', $pm->getFreePort(), SWOOLE_BASE);
     $server->data = array();
 
     $server->setHandler('GET', function ($fd, $data) use ($server) {
-        if (count($data) == 0)
-        {
+        if (count($data) == 0) {
             return Server::format(Server::ERROR, "ERR wrong number of arguments for 'GET' command");
         }
         $key = $data[0];
-        if (empty($server->data[$key]))
-        {
+        if (empty($server->data[$key])) {
             $server->send($fd, Server::format(Server::NIL));
-        }
-        else
-        {
+        } else {
             $server->send($fd, Server::format(Server::STRING, $server->data[$key]));
         }
     });
 
     $server->setHandler('SET', function ($fd, $data) use ($server) {
-        if (count($data) < 2)
-        {
+        if (count($data) < 2) {
             return Server::format(Server::ERROR, "ERR wrong number of arguments for 'SET' command");
         }
         $key = $data[0];
-Original file line number
+Diff line change
@@ Expand Up / @@ -121,7 +121,7 @@ function swoole_internal_call_user_shutdown_begin(): bool @@
     }
-    function swoole_implicit_fn(string $fn, mixed $args = null): void
+    function swoole_implicit_fn(string $fn, mixed $args = null): mixed
     {
     }