From 6389c692d5041a259e7a285f8f647ddcba88d2c2 Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Mon, 4 Dec 2023 15:08:33 -0700 Subject: [PATCH] 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 Co-authored-by: bduranleau-nr <106178551+bduranleau-nr@users.noreply.github.com> --- agent/fw_drupal.c | 4 ++++ agent/fw_drupal8.c | 5 ++++- agent/fw_wordpress.c | 13 +++++++++++++ agent/php_newrelic.h | 3 +++ agent/php_rinit.c | 2 ++ agent/php_vm.c | 8 ++++++++ agent/tests/test_internal_instrument.c | 2 ++ 7 files changed, 36 insertions(+), 1 deletion(-) diff --git a/agent/fw_drupal.c b/agent/fw_drupal.c index 13fcc68a9..5305ab480 100644 --- 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 index 05849031a..77695e058 100644 --- 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 index ad67b9368..2c419b7ef 100644 --- 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 index 0baed648b..85887fe29 100644 --- 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 index e98c5b198..c8045f2e1 100644 --- 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 index 19f4bb215..4f8d7c023 100644 --- 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 index de3f19619..0f4f2b3f8 100644 --- 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', "