refactor: only call cufa callback when needed (#769)

Improve agent's performance by only calling `call_user_function_array` callback when trying to instrument Drupal or WordPress hooks. Imported from #729. --------- Co-authored-by: Michal Nowacki <[email protected]> Co-authored-by: bduranleau-nr <[email protected]>
newrelic · Dec 4, 2023 · 6389c69 · 6389c69
1 parent 54ad95d
commit 6389c69
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 1 deletion.
diff --git a/agent/fw_drupal.c b/agent/fw_drupal.c
@@ -608,12 +608,16 @@ NR_PHP_WRAPPER(nr_drupal_wrap_module_invoke_all) {
   NRPRG(drupal_module_invoke_all_hook)
       = nr_strndup(Z_STRVAL_P(hook), Z_STRLEN_P(hook));
   NRPRG(drupal_module_invoke_all_hook_len) = Z_STRLEN_P(hook);
+  NRPRG(check_cufa) = true;
 
   NR_PHP_WRAPPER_CALL;
 
   nr_free(NRPRG(drupal_module_invoke_all_hook));
   NRPRG(drupal_module_invoke_all_hook) = prev_hook;
   NRPRG(drupal_module_invoke_all_hook_len) = prev_hook_len;
+  if (NULL == NRPRG(drupal_module_invoke_all_hook)) {
+    NRPRG(check_cufa) = false;
+  }
 
 leave:
   nr_php_arg_release(&hook);

diff --git a/agent/fw_drupal8.c b/agent/fw_drupal8.c
@@ -418,7 +418,7 @@ NR_PHP_WRAPPER(nr_drupal94_invoke_all_with) {
   NRPRG(drupal_module_invoke_all_hook)
       = nr_strndup(Z_STRVAL_P(hook), Z_STRLEN_P(hook));
   NRPRG(drupal_module_invoke_all_hook_len) = Z_STRLEN_P(hook);
-
+  NRPRG(check_cufa) = true;
   callback = nr_php_arg_get(2, NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
   /* This instrumentation will fail if callback has already been wrapped
    * with a special instrumentation callback in a different context.
@@ -432,6 +432,9 @@ NR_PHP_WRAPPER(nr_drupal94_invoke_all_with) {
   nr_free(NRPRG(drupal_module_invoke_all_hook));
   NRPRG(drupal_module_invoke_all_hook) = prev_hook;
   NRPRG(drupal_module_invoke_all_hook_len) = prev_hook_len;
+  if (NULL == NRPRG(drupal_module_invoke_all_hook)) {
+    NRPRG(check_cufa) = false;
+  }
 
 leave:
   nr_php_arg_release(&hook);

diff --git a/agent/fw_wordpress.c b/agent/fw_wordpress.c
@@ -494,10 +494,16 @@ NR_PHP_WRAPPER(nr_wordpress_exec_handle_tag) {
      */
     char* old_tag = NRPRG(wordpress_tag);
 
+    NRPRG(check_cufa) = true;
+
     NRPRG(wordpress_tag) = nr_wordpress_clean_tag(tag);
     NR_PHP_WRAPPER_CALL;
     NRPRG(wordpress_tag) = old_tag;
+    if (NULL == NRPRG(wordpress_tag)) {
+      NRPRG(check_cufa) = false;
+    }
   } else {
+    NRPRG(check_cufa) = false;
     NR_PHP_WRAPPER_CALL;
   }
 
@@ -575,10 +581,17 @@ NR_PHP_WRAPPER(nr_wordpress_apply_filters) {
        */
       char* old_tag = NRPRG(wordpress_tag);
 
+      NRPRG(check_cufa) = true;
+
       NRPRG(wordpress_tag) = nr_wordpress_clean_tag(tag);
+
       NR_PHP_WRAPPER_CALL;
       NRPRG(wordpress_tag) = old_tag;
+      if (NULL == NRPRG(wordpress_tag)) {
+        NRPRG(check_cufa) = false;
+      }
     } else {
+      NRPRG(check_cufa) = false;
       NR_PHP_WRAPPER_CALL;
     }
 

diff --git a/agent/php_newrelic.h b/agent/php_newrelic.h
@@ -378,6 +378,9 @@ int symfony1_in_dispatch; /* Whether we are currently within a
 int symfony1_in_error404; /* Whether we are currently within a
                              sfError404Exception::printStackTrace() frame */
 
+bool check_cufa; /* Whether we need to check cufa because we are
+                    instrumenting hooks, or whether we can skip cufa */
+
 char* wordpress_tag;                   /* The current WordPress tag */
 nr_regex_t* wordpress_plugin_regex;    /* Regex for plugin filenames */
 nr_regex_t* wordpress_theme_regex;     /* Regex for theme filenames */

diff --git a/agent/php_rinit.c b/agent/php_rinit.c
@@ -89,6 +89,8 @@ PHP_RINIT_FUNCTION(newrelic) {
     nr_php_extension_instrument_rescan(NRPRG(extensions) TSRMLS_CC);
   }
 
+  NRPRG(check_cufa) = false;
+
   NRPRG(mysql_last_conn) = NULL;
   NRPRG(pgsql_last_conn) = NULL;
   NRPRG(datastore_connections) = nr_hashmap_create(

diff --git a/agent/php_vm.c b/agent/php_vm.c
@@ -108,6 +108,14 @@ static int nr_php_handle_cufa_fcall(zend_execute_data* execute_data) {
     goto call_previous_and_return;
   }
 
+  /*
+   * If we don't have instrumented hooks that require this, skip to the
+   * end.
+   */
+  if (false == NRPRG(check_cufa)) {
+    goto call_previous_and_return;
+  }
+
   /*
    * Since we're in the middle of a function call, the Zend Engine is actually
    * only partway through constructing the new function frame. As a result, it

diff --git a/agent/tests/test_internal_instrument.c b/agent/tests/test_internal_instrument.c
@@ -49,6 +49,7 @@ static void test_cufa_direct(TSRMLS_D) {
 
   tlib_php_request_start();
 
+  NRPRG(check_cufa) = true;
   define_cufa_function_f(TSRMLS_C);
   tlib_php_request_eval(
       "function g() { return call_user_func_array('f', array()); }" TSRMLS_CC);
@@ -74,6 +75,7 @@ static void test_cufa_indirect(TSRMLS_D) {
 
   tlib_php_request_start();
 
+  NRPRG(check_cufa) = true;
   define_cufa_function_f(TSRMLS_C);
   tlib_php_request_eval(
       "function g() { $cufa = 'call_user_func_array'; return $cufa('f', "