Optimize the implementation of callbacks and callables. (#5494)

* Optimize zend_fci_cache code; Remove swoole_add_function/swoole_get_function; Enforce single-threading when using package_length_func and dispatch_func.

* fix tests

* fix tests [2], filter=unit

* refactor, --filter=unit

* optimize code, --filter=[unit]

* fix, --filter=[unit][thread]

* fix 2, --filter=[unit][thread]

* fix 3, --filter=[unit]

* fix 4, --filter=[unit]

* fix 5

core-tests/src/core/base.cpp

@@ -223,17 +223,6 @@ static std::string test_func(std::string test_data_2) {
     return test_data + test_data_2;
 }
 
-TEST(base, add_function) {
-    typedef std::string (*_func_t)(std::string);
-    swoole_add_function("test_func", (void *) test_func);
-    ASSERT_EQ(swoole_add_function("test_func", (void *) test_func), SW_ERR);
-    _func_t _func = (_func_t) swoole_get_function(SW_STRL("test_func"));
-    std::string b = ", swoole is best";
-    auto rs = _func(", swoole is best");
-    ASSERT_EQ(rs, test_data + b);
-    ASSERT_EQ(swoole_get_function(SW_STRL("test_func31")), nullptr);
-}
-
 TEST(base, hook) {
     int count = 0;
     swoole_add_hook(

ext-src/php_swoole_cxx.h

@@ -570,32 +570,70 @@ class CharPtr {
     }
 };
 
-struct Callable {
-    zval zfunc;
+class Callable {
+  private:
+    zval zfn;
     zend_fcall_info_cache fcc;
+    char *fn_name = nullptr;
+
+    Callable() {}
+
+  public:
+    Callable(zval *_zfn) {
+        ZVAL_UNDEF(&zfn);
+        if (!zval_is_true(_zfn)) {
+            php_swoole_fatal_error(E_WARNING, "illegal callback function");
+            return;
+        }
+        if (!sw_zend_is_callable_ex(_zfn, nullptr, 0, &fn_name, nullptr, &fcc, nullptr)) {
+            php_swoole_fatal_error(E_WARNING, "function '%s' is not callable", fn_name);
+            return;
+        }
+        zfn = *_zfn;
+        zval_add_ref(&zfn);
+    }
+
+    zend_fcall_info_cache *ptr() {
+        return &fcc;
+    }
 
-    Callable(zval *_zfunc) {
-        zfunc = *_zfunc;
-        Z_TRY_ADDREF_P(&zfunc);
+    bool ready() {
+        return !ZVAL_IS_UNDEF(&zfn);
     }
 
-    bool is_callable() {
-        return zend_is_callable_ex(&zfunc, NULL, 0, NULL, &fcc, NULL);
+    Callable *dup() {
+        auto copy = new Callable();
+        copy->fcc = fcc;
+        copy->zfn = zfn;
+        zval_add_ref(&copy->zfn);
+        if (fn_name) {
+            copy->fn_name = estrdup(fn_name);
+        }
+        return copy;
     }
 
     bool call(uint32_t argc, zval *argv, zval *retval) {
-        return sw_zend_call_function_ex(&zfunc, &fcc, argc, argv, retval) == SUCCESS;
+        return sw_zend_call_function_ex(&zfn, &fcc, argc, argv, retval) == SUCCESS;
     }
 
     ~Callable() {
-        Z_TRY_DELREF_P(&zfunc);
+        if (!ZVAL_IS_UNDEF(&zfn)) {
+            zval_ptr_dtor(&zfn);
+        }
+        if (fn_name) {
+            efree(fn_name);
+        }
     }
 };
 
 namespace function {
 /* must use this API to call event callbacks to ensure that exceptions are handled correctly */
 bool call(zend_fcall_info_cache *fci_cache, uint32_t argc, zval *argv, zval *retval, const bool enable_coroutine);
 Variable call(const std::string &func_name, int argc, zval *argv);
+
+static inline bool call(Callable *cb, uint32_t argc, zval *argv, zval *retval, const bool enable_coroutine) {
+    return call(cb->ptr(), argc, argv, retval, enable_coroutine);
+}
 }  // namespace function
 
 struct Function {
@@ -689,17 +727,31 @@ static inline void print_error(zend_object *exception, int severity) {
 //-----------------------------------namespace end--------------------------------------------
 }  // namespace zend
 
-static inline zend::Callable *php_swoole_zval_to_callable(zval *zfn, const char *fname, bool allow_null = true) {
+/* use void* to match some C callback function pointers */
+static inline void sw_callable_free(void *ptr) {
+    delete (zend::Callable *) ptr;
+}
+
+static inline zend::Callable *sw_callable_create(zval *zfn) {
+    auto fn = new zend::Callable(zfn);
+    if (fn->ready()) {
+        return fn;
+    } else {
+        delete fn;
+        return nullptr;
+    }
+}
+
+static inline zend::Callable *sw_callable_create_ex(zval *zfn, const char *fname, bool allow_null = true) {
     if (zfn == nullptr || ZVAL_IS_NULL(zfn)) {
         if (!allow_null) {
             zend_throw_exception_ex(
                 swoole_exception_ce, SW_ERROR_INVALID_PARAMS, "%s must be of type callable, null given", fname);
         }
         return nullptr;
     }
-    auto cb = new zend::Callable(zfn);
-    if (!cb->is_callable()) {
-        delete cb;
+    auto cb = sw_callable_create(zfn);
+    if (!cb) {
         zend_throw_exception_ex(swoole_exception_ce,
                                 SW_ERROR_INVALID_PARAMS,
                                 "%s must be of type callable, %s given",
@@ -709,8 +761,3 @@ static inline zend::Callable *php_swoole_zval_to_callable(zval *zfn, const char
     }
     return cb;
 }
-
-static inline void php_swoole_callable_free(void *ptr) {
-    zend::Callable *cb = (zend::Callable *) ptr;
-    delete cb;
-}

ext-src/php_swoole_private.h

@@ -972,12 +972,6 @@ static sw_inline void sw_zend_fci_cache_discard(zend_fcall_info_cache *fci_cache
     }
 }
 
-/* use void* to match some C callback function pointers */
-static sw_inline void sw_zend_fci_cache_free(void *fci_cache) {
-    sw_zend_fci_cache_discard((zend_fcall_info_cache *) fci_cache);
-    efree((zend_fcall_info_cache *) fci_cache);
-}
-
 #if PHP_VERSION_ID >= 80100
 #define sw_php_spl_object_hash(o) php_spl_object_hash(Z_OBJ_P(o))
 #else

ext-src/php_swoole_server.h

@@ -73,9 +73,7 @@ void php_swoole_server_set_port_property(swoole::ListenPort *port, swoole::Serve
 namespace swoole {
 
 struct ServerPortProperty {
-    zval *callbacks[PHP_SWOOLE_SERVER_PORT_CALLBACK_NUM];
-    zend_fcall_info_cache *caches[PHP_SWOOLE_SERVER_PORT_CALLBACK_NUM];
-    zval _callbacks[PHP_SWOOLE_SERVER_PORT_CALLBACK_NUM];
+    zend::Callable *callbacks[PHP_SWOOLE_SERVER_PORT_CALLBACK_NUM];
     Server *serv;
     ListenPort *port;
     zval *zsetting;
@@ -84,11 +82,11 @@ struct ServerPortProperty {
 struct ServerProperty {
     std::vector<zval *> ports;
     std::vector<zval *> user_processes;
-    zend_fcall_info_cache *callbacks[PHP_SWOOLE_SERVER_CALLBACK_NUM];
-    std::unordered_map<TaskId, zend_fcall_info_cache> task_callbacks;
+    zend::Callable *callbacks[PHP_SWOOLE_SERVER_CALLBACK_NUM];
+    std::unordered_map<TaskId, zend::Callable *> task_callbacks;
     std::unordered_map<TaskId, TaskCo *> task_coroutine_map;
     std::unordered_map<SessionId, std::list<Coroutine *> *> send_coroutine_map;
-    std::vector<zend_fcall_info_cache *> command_callbacks;
+    std::vector<zend::Callable *> command_callbacks;
 };
 
 struct ServerObject {
@@ -106,6 +104,14 @@ struct ServerObject {
                 php_swoole_server_get_port_property(serv->get_primary_port())->callbacks[event_type]);
     }
 
+    bool isset_callback(int event_type) {
+        return property->callbacks[event_type] != nullptr;
+    }
+
+    zend::Callable *get_callback(int event_type) {
+        return property->callbacks[event_type];
+    }
+
     zend_bool is_websocket_server() {
         return instanceof_function(get_ce(), swoole_websocket_server_ce);
     }
@@ -132,7 +138,7 @@ void register_admin_server_commands(Server *serv);
 }  // namespace swoole
 
 void php_swoole_server_register_callbacks(swServer *serv);
-zend_fcall_info_cache *php_swoole_server_get_fci_cache(swServer *serv, int server_fd, int event_type);
+zend::Callable *php_swoole_server_get_callback(swServer *serv, int server_fd, int event_type);
 int php_swoole_create_dir(const char *path, size_t length);
 void php_swoole_server_before_start(swServer *serv, zval *zobject);
 bool php_swoole_server_isset_callback(swServer *serv, swListenPort *port, int event_type);

ext-src/swoole_client.cc

@@ -34,19 +34,6 @@ using swoole::String;
 using swoole::network::Client;
 using swoole::network::Socket;
 
-struct ClientCallback {
-    zend_fcall_info_cache cache_onConnect;
-    zend_fcall_info_cache cache_onReceive;
-    zend_fcall_info_cache cache_onClose;
-    zend_fcall_info_cache cache_onError;
-    zend_fcall_info_cache cache_onBufferFull;
-    zend_fcall_info_cache cache_onBufferEmpty;
-#ifdef SW_USE_OPENSSL
-    zend_fcall_info_cache cache_onSSLReady;
-#endif
-    zval _object;
-};
-
 static std::unordered_map<std::string, std::queue<Client *> *> long_connections;
 
 zend_class_entry *swoole_client_ce;
@@ -58,7 +45,6 @@ static zend_object_handlers swoole_client_exception_handlers;
 struct ClientObject {
     Client *cli;
     zval *zsocket;
-    ClientCallback *cb;
     zend_object std;
 };
 
@@ -87,14 +73,6 @@ static sw_inline void php_swoole_client_set_zsocket(zval *zobject, zval *zsocket
 }
 #endif
 
-static sw_inline ClientCallback *php_swoole_client_get_cb(zval *zobject) {
-    return php_swoole_client_fetch_object(Z_OBJ_P(zobject))->cb;
-}
-
-static sw_inline void php_swoole_client_set_cb(zval *zobject, ClientCallback *cb) {
-    php_swoole_client_fetch_object(Z_OBJ_P(zobject))->cb = cb;
-}
-
 static void php_swoole_client_free_object(zend_object *object) {
     zend_object_std_dtor(object);
 }
@@ -352,32 +330,15 @@ bool php_swoole_client_check_setting(Client *cli, zval *zset) {
     }
     // length function
     if (php_swoole_array_get_value(vht, "package_length_func", ztmp)) {
-        while (1) {
-            if (Z_TYPE_P(ztmp) == IS_STRING) {
-                Protocol::LengthFunc func = Protocol::get_function(std::string(Z_STRVAL_P(ztmp), Z_STRLEN_P(ztmp)));
-                if (func != nullptr) {
-                    cli->protocol.get_package_length = func;
-                    break;
-                }
-            }
-
-            char *func_name;
-            zend_fcall_info_cache *fci_cache = (zend_fcall_info_cache *) ecalloc(1, sizeof(zend_fcall_info_cache));
-            if (!sw_zend_is_callable_ex(ztmp, nullptr, 0, &func_name, nullptr, fci_cache, nullptr)) {
-                php_swoole_fatal_error(E_ERROR, "function '%s' is not callable", func_name);
-                return false;
-            }
-            efree(func_name);
-            cli->protocol.get_package_length = php_swoole_length_func;
-            if (cli->protocol.private_data) {
-                sw_zend_fci_cache_discard((zend_fcall_info_cache *) cli->protocol.private_data);
-                efree(cli->protocol.private_data);
-            }
-            sw_zend_fci_cache_persist(fci_cache);
-            cli->protocol.private_data = fci_cache;
-            break;
+        auto fci_cache = sw_callable_create(ztmp);
+        if (!fci_cache) {
+            return false;
         }
-
+        cli->protocol.get_package_length = php_swoole_length_func;
+        if (cli->protocol.private_data_1) {
+            sw_callable_free(cli->protocol.private_data_1);
+        }
+        cli->protocol.private_data_1 = fci_cache;
         cli->protocol.package_length_size = 0;
         cli->protocol.package_length_type = '\0';
         cli->protocol.package_length_offset = SW_IPC_BUFFER_SIZE;
@@ -526,10 +487,9 @@ static void php_swoole_client_free(zval *zobject, Client *cli) {
         swoole_timer_del(cli->timer);
         cli->timer = nullptr;
     }
-    if (cli->protocol.private_data) {
-        sw_zend_fci_cache_discard((zend_fcall_info_cache *) cli->protocol.private_data);
-        efree(cli->protocol.private_data);
-        cli->protocol.private_data = nullptr;
+    if (cli->protocol.private_data_1) {
+        sw_callable_free(cli->protocol.private_data_1);
+        cli->protocol.private_data_1 = nullptr;
     }
     // long tcp connection, delete from php_sw_long_connections
     if (cli->keep) {
@@ -558,14 +518,14 @@ static void php_swoole_client_free(zval *zobject, Client *cli) {
 }
 
 ssize_t php_swoole_length_func(const Protocol *protocol, Socket *_socket, PacketLength *pl) {
-    zend_fcall_info_cache *fci_cache = (zend_fcall_info_cache *) protocol->private_data;
+    zend::Callable *cb = (zend::Callable *) protocol->private_data_1;
     zval zdata;
     zval retval;
     ssize_t ret = -1;
 
     // TODO: reduce memory copy
     ZVAL_STRINGL(&zdata, pl->buf, pl->buf_size);
-    if (UNEXPECTED(sw_zend_call_function_ex2(nullptr, fci_cache, 1, &zdata, &retval) != SUCCESS)) {
+    if (UNEXPECTED(sw_zend_call_function_ex2(nullptr, cb->ptr(), 1, &zdata, &retval) != SUCCESS)) {
         php_swoole_fatal_error(E_WARNING, "length function handler error");
     } else {
         ret = zval_get_long(&retval);
@@ -685,7 +645,6 @@ static PHP_METHOD(swoole_client, __construct) {
     }
     // init
     php_swoole_client_set_cli(ZEND_THIS, nullptr);
-    php_swoole_client_set_cb(ZEND_THIS, nullptr);
 #ifdef SWOOLE_SOCKETS_SUPPORT
     php_swoole_client_set_zsocket(ZEND_THIS, nullptr);
 #endif
@@ -700,12 +659,6 @@ static PHP_METHOD(swoole_client, __destruct) {
     if (cli) {
         sw_zend_call_method_with_0_params(ZEND_THIS, swoole_client_ce, nullptr, "close", nullptr);
     }
-    // free memory
-    ClientCallback *cb = php_swoole_client_get_cb(ZEND_THIS);
-    if (cb) {
-        efree(cb);
-        php_swoole_client_set_cb(ZEND_THIS, nullptr);
-    }
 }
 
 static PHP_METHOD(swoole_client, set) {

ext-src/swoole_client_coro.cc

@@ -131,10 +131,9 @@ static zend_object *client_coro_create_object(zend_class_entry *ce) {
 }
 
 static void client_coro_socket_dtor(ClientCoroObject *client) {
-    if (client->socket->protocol.private_data) {
-        sw_zend_fci_cache_discard((zend_fcall_info_cache *) client->socket->protocol.private_data);
-        efree(client->socket->protocol.private_data);
-        client->socket->protocol.private_data = nullptr;
+    if (client->socket->protocol.private_data_1) {
+        sw_callable_free(client->socket->protocol.private_data_1);
+        client->socket->protocol.private_data_1 = nullptr;
     }
     client->socket = nullptr;
     zend_update_property_null(Z_OBJCE_P(&client->zobject), SW_Z8_OBJ_P(&client->zobject), ZEND_STRL("socket"));