Skip to content

Commit f6b2914

Browse files
lavarouzsistla
andauthored
refactor(agent): optimize user function instrumentation lookup for PHPs 8.0+ (#1042)
Reduce agent’s CPU overhead when observer API is used to hook into Zend Engine. This is achieved by: * Simplifying the checks the agent needs to perform to determine if special instrumentation for user function is provided. Before this change, on every function execution the agent used a hashmap to check if special instrumentation for user function is provided. However, when the observer API is used to hook into Zend Engine, this can be done only once per user function execution at the time when the user function is executed for the first time. * Reducing the number of times the name of the file is compared with the list of ‘magic files’ to detect package and/or framework used. Before this change on every file execution the agent scanned the list of magic files twice. However, when the observer API is used to hook into Zend Engine, this can be done only once per file execution. --------- Co-authored-by: Amber Sistla <[email protected]>
1 parent 5a897c8 commit f6b2914

40 files changed

+2194
-115
lines changed

agent/Makefile.frag

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ TEST_BINARIES = \
117117
tests/test_txn_private \
118118
tests/test_user_instrument \
119119
tests/test_user_instrument_hashmap \
120+
tests/test_user_instrument_wraprec_hashmap \
120121
tests/test_zval
121122

122123
.PHONY: unit-tests

agent/config.m4

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ if test "$PHP_NEWRELIC" = "yes"; then
220220
php_pdo_mysql.c php_pdo_pgsql.c php_pgsql.c php_psr7.c php_redis.c \
221221
php_rinit.c php_rshutdown.c php_samplers.c php_stack.c \
222222
php_stacked_segment.c php_txn.c php_user_instrument.c \
223+
php_user_instrument_wraprec_hashmap.c \
223224
php_user_instrument_hashmap.c php_vm.c php_wrapper.c"
224225
FRAMEWORKS="fw_cakephp.c fw_codeigniter.c fw_drupal8.c \
225226
fw_drupal.c fw_drupal_common.c fw_joomla.c fw_kohana.c \

agent/fw_drupal8.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,14 +140,17 @@ static void nr_drupal8_add_method_callback_before_after_clean(
140140
nrspecialfn_t after_callback,
141141
nrspecialfn_t clean_callback) {
142142
zend_function* function = NULL;
143+
char* methodLC = NULL;
143144

144145
if (NULL == ce) {
145146
nrl_verbosedebug(NRL_FRAMEWORK, "Drupal 8: got NULL class entry in %s",
146147
__func__);
147148
return;
148149
}
149150

150-
function = nr_php_find_class_method(ce, method);
151+
methodLC = nr_string_to_lowercase(method);
152+
function = nr_php_find_class_method(ce, methodLC);
153+
nr_free(methodLC);
151154
if (NULL == function) {
152155
nrl_verbosedebug(NRL_FRAMEWORK,
153156
"Drupal 8+: cannot get zend_function entry for %.*s::%.*s",
@@ -640,7 +643,7 @@ NR_PHP_WRAPPER(nr_drupal8_module_handler) {
640643
#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
641644
&& !defined OVERWRITE_ZEND_EXECUTE_DATA
642645
nr_drupal8_add_method_callback_before_after_clean(
643-
ce, NR_PSTR("invokeallwith"), nr_drupal94_invoke_all_with,
646+
ce, NR_PSTR("invokeAllWith"), nr_drupal94_invoke_all_with,
644647
nr_drupal94_invoke_all_with_after, nr_drupal94_invoke_all_with_clean);
645648
#else
646649
nr_drupal8_add_method_callback(ce, NR_PSTR("invokeallwith"),

agent/fw_laminas3.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ void nr_laminas3_enable(TSRMLS_D) {
155155
*/
156156

157157
nr_php_wrap_user_function(
158-
NR_PSTR("Laminas\\Router\\HTTP\\RouteMatch::setMatchedRouteName"),
158+
NR_PSTR("Laminas\\Router\\Http\\RouteMatch::setMatchedRouteName"),
159159
nr_laminas3_name_the_wt TSRMLS_CC);
160160
nr_php_wrap_user_function(
161161
NR_PSTR("Laminas\\Router\\RouteMatch::setMatchedRouteName"),

agent/fw_laravel.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1232,10 +1232,10 @@ void nr_laravel_enable(TSRMLS_D) {
12321232
#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
12331233
&& !defined OVERWRITE_ZEND_EXECUTE_DATA
12341234
nr_php_wrap_user_function_before_after_clean(
1235-
NR_PSTR("Illuminate\\Console\\Application::doRun"),
1235+
NR_PSTR("Symfony\\Component\\Console\\Application::doRun"),
12361236
nr_laravel_console_application_dorun, NULL, NULL);
12371237
#else
1238-
nr_php_wrap_user_function(NR_PSTR("Illuminate\\Console\\Application::doRun"),
1238+
nr_php_wrap_user_function(NR_PSTR("Symfony\\Component\\Console\\Application::doRun"),
12391239
nr_laravel_console_application_dorun TSRMLS_CC);
12401240
#endif
12411241
/*

agent/lib_php_amqplib.c

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -81,29 +81,6 @@
8181
* needed to work with MQ_BROKER.
8282
*/
8383

84-
/*
85-
* Purpose : Ensures the php-amqplib instrumentation gets wrapped.
86-
*
87-
* Params : None
88-
*
89-
* Returns : None
90-
*/
91-
static void nr_php_amqplib_ensure_class() {
92-
int result = FAILURE;
93-
zend_class_entry* class_entry = NULL;
94-
95-
class_entry = nr_php_find_class("phpamqplib\\channel\\amqpchannel");
96-
if (NULL == class_entry) {
97-
result = zend_eval_stringl(
98-
NR_PSTR("class_exists('PhpAmqpLib\\Channel\\AMQPChannel');"), NULL,
99-
"nr_php_amqplib_class_exists_channel_amqpchannel");
100-
}
101-
/*
102-
* We don't need to check anything else at this point. If this fails, there's
103-
* nothing else we can do anyway.
104-
*/
105-
}
106-
10784
/*
10885
* Version information will be pulled from PhpAmqpLib\\Package::VERSION
10986
* nr_php_amqplib_handle_version will automatically load the class if it isn't
@@ -810,7 +787,6 @@ void nr_php_amqplib_enable() {
810787

811788
/* Extract the version */
812789
nr_php_amqplib_handle_version();
813-
nr_php_amqplib_ensure_class();
814790

815791
#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* less than PHP8.0 */
816792
nr_php_wrap_user_function_before_after_clean(

agent/php_execute.c

Lines changed: 17 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@
9595

9696
static void nr_php_show_exec_return(NR_EXECUTE_PROTO TSRMLS_DC);
9797
static int nr_php_show_exec_indentation(TSRMLS_D);
98-
static void nr_php_show_exec(NR_EXECUTE_PROTO TSRMLS_DC);
9998

10099
/*
101100
* Purpose: Enable monitoring on specific functions in the framework.
@@ -594,11 +593,11 @@ static int nr_php_show_exec_indentation(TSRMLS_D) {
594593
* Note that this function doesn't handle internal functions, and will crash if
595594
* you give it one.
596595
*/
597-
static void nr_php_show_exec(NR_EXECUTE_PROTO TSRMLS_DC) {
596+
void nr_php_show_exec(const char* context, NR_EXECUTE_PROTO TSRMLS_DC) {
598597
char argstr[NR_EXECUTE_DEBUG_STRBUFSZ];
599598
const char* filename = nr_php_op_array_file_name(NR_OP_ARRAY);
600599
const char* function_name = nr_php_op_array_function_name(NR_OP_ARRAY);
601-
600+
const char* ctx = context ? context : "execute";
602601
argstr[0] = '\0';
603602

604603
if (NR_OP_ARRAY->scope) {
@@ -608,12 +607,13 @@ static void nr_php_show_exec(NR_EXECUTE_PROTO TSRMLS_DC) {
608607
nr_show_execute_params(NR_EXECUTE_ORIG_ARGS, argstr TSRMLS_CC);
609608
nrl_verbosedebug(
610609
NRL_AGENT,
611-
"execute: %.*s scope={%.*s} function={" NRP_FMT_UQ
610+
NRP_FMT_UQ ": %.*s scope={%.*s} function={" NRP_FMT_UQ
612611
"}"
613612
" params={" NRP_FMT_UQ
614613
"}"
615614
" %.5s"
616615
"@ " NRP_FMT_UQ ":%d",
616+
NRP_SHOW_EXEC_CONTEXT(ctx),
617617
nr_php_show_exec_indentation(TSRMLS_C), nr_php_indentation_spaces,
618618
NRSAFELEN(nr_php_class_entry_name_length(NR_OP_ARRAY->scope)),
619619
nr_php_class_entry_name(NR_OP_ARRAY->scope),
@@ -631,12 +631,13 @@ static void nr_php_show_exec(NR_EXECUTE_PROTO TSRMLS_DC) {
631631
nr_show_execute_params(NR_EXECUTE_ORIG_ARGS, argstr TSRMLS_CC);
632632
nrl_verbosedebug(
633633
NRL_AGENT,
634-
"execute: %.*s function={" NRP_FMT_UQ
634+
NRP_FMT_UQ ": %.*s function={" NRP_FMT_UQ
635635
"}"
636636
" params={" NRP_FMT_UQ
637637
"}"
638638
" %.5s"
639639
"@ " NRP_FMT_UQ ":%d",
640+
NRP_SHOW_EXEC_CONTEXT(ctx),
640641
nr_php_show_exec_indentation(TSRMLS_C), nr_php_indentation_spaces,
641642
NRP_PHP(function_name), NRP_ARGSTR(argstr),
642643
#if ZEND_MODULE_API_NO < ZEND_7_4_X_API_NO
@@ -649,16 +650,17 @@ static void nr_php_show_exec(NR_EXECUTE_PROTO TSRMLS_DC) {
649650
/*
650651
* file
651652
*/
652-
nrl_verbosedebug(NRL_AGENT, "execute: %.*s file={" NRP_FMT "}",
653+
nrl_verbosedebug(NRL_AGENT, NRP_FMT_UQ ": %.*s file={" NRP_FMT "}",
654+
NRP_SHOW_EXEC_CONTEXT(ctx),
653655
nr_php_show_exec_indentation(TSRMLS_C),
654656
nr_php_indentation_spaces, NRP_FILENAME(filename));
655657
} else {
656658
/*
657659
* unknown
658660
*/
659-
nrl_verbosedebug(NRL_AGENT, "execute: %.*s ?",
660-
nr_php_show_exec_indentation(TSRMLS_C),
661-
nr_php_indentation_spaces);
661+
nrl_verbosedebug(NRL_AGENT, NRP_FMT_UQ ": %.*s ?",
662+
NRP_SHOW_EXEC_CONTEXT(ctx),
663+
nr_php_show_exec_indentation(TSRMLS_C), nr_php_indentation_spaces);
662664
}
663665
}
664666

@@ -982,7 +984,7 @@ static void nr_php_user_instrumentation_from_file(const char* filename,
982984
*/
983985
#define METRIC_NAME_MAX_LEN 512
984986

985-
static void nr_php_execute_file(const zend_op_array* op_array,
987+
void nr_php_execute_file(const zend_op_array* op_array,
986988
NR_EXECUTE_PROTO TSRMLS_DC) {
987989
const char* filename = nr_php_op_array_file_name(op_array);
988990
size_t filename_len = nr_php_op_array_file_name_len(op_array);
@@ -1007,7 +1009,9 @@ static void nr_php_execute_file(const zend_op_array* op_array,
10071009
return;
10081010
}
10091011

1012+
#if ZEND_MODULE_API_NO < ZEND_8_0_X_API_NO
10101013
nr_php_add_user_instrumentation(TSRMLS_C);
1014+
#endif
10111015
}
10121016

10131017
/*
@@ -1519,7 +1523,7 @@ static void nr_php_execute_enabled(NR_EXECUTE_PROTO TSRMLS_DC) {
15191523

15201524
static void nr_php_execute_show(NR_EXECUTE_PROTO TSRMLS_DC) {
15211525
if (nrunlikely(NR_PHP_PROCESS_GLOBALS(special_flags).show_executes)) {
1522-
nr_php_show_exec(NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
1526+
nr_php_show_exec("execute", NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
15231527
}
15241528

15251529
nr_php_execute_enabled(NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
@@ -1905,16 +1909,7 @@ static void nr_php_instrument_func_begin(NR_EXECUTE_PROTO) {
19051909

19061910
NRTXNGLOBAL(execute_count) += 1;
19071911
txn_start_time = nr_txn_start_time(NRPRG(txn));
1908-
/*
1909-
* Handle here, but be aware the classes might not be loaded yet.
1910-
*/
1911-
if (nrunlikely(OP_ARRAY_IS_A_FILE(NR_OP_ARRAY))) {
1912-
const char* filename = nr_php_op_array_file_name(NR_OP_ARRAY);
1913-
size_t filename_len = nr_php_op_array_file_name_len(NR_OP_ARRAY);
1914-
nr_execute_handle_framework(all_frameworks, num_all_frameworks,
1915-
filename, filename_len TSRMLS_CC);
1916-
return;
1917-
}
1912+
19181913
if (NULL != NRPRG(cufa_callback) && NRPRG(check_cufa)) {
19191914
/*
19201915
* For PHP 7+, call_user_func_array() is flattened into an inline by
@@ -2000,14 +1995,6 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO) {
20001995
}
20011996
txn_start_time = nr_txn_start_time(NRPRG(txn));
20021997

2003-
/*
2004-
* Let's get the framework info.
2005-
*/
2006-
if (nrunlikely(OP_ARRAY_IS_A_FILE(NR_OP_ARRAY))) {
2007-
nr_php_execute_file(NR_OP_ARRAY, NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
2008-
return;
2009-
}
2010-
20111998
/*
20121999
* Get the current segment and return if null.
20132000
*/
@@ -2157,7 +2144,7 @@ void nr_php_observer_fcall_begin(zend_execute_data* execute_data) {
21572144
int show_executes = NR_PHP_PROCESS_GLOBALS(special_flags).show_executes;
21582145

21592146
if (nrunlikely(show_executes)) {
2160-
nr_php_show_exec(NR_EXECUTE_ORIG_ARGS);
2147+
nr_php_show_exec("execute", NR_EXECUTE_ORIG_ARGS);
21612148
}
21622149
nr_php_instrument_func_begin(NR_EXECUTE_ORIG_ARGS);
21632150

agent/php_execute.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,21 @@
2727
#define OP_ARRAY_IS_METHOD(OP, FNAME) \
2828
(0 == nr_strcmp(nr_php_op_array_function_name(OP), (FNAME)))
2929

30+
extern void nr_php_execute_file(const zend_op_array* op_array,
31+
NR_EXECUTE_PROTO TSRMLS_DC);
32+
33+
/*
34+
* Purpose: Log information about the execute data in a given execution
35+
* context - either 'execute' (zend_execute) or 'observe' (fcall_init).
36+
* Only first 8 characters of the context are printed.
37+
*
38+
* Caveat: This function doesn't handle internal functions, and will crash if
39+
* you give it one.
40+
*/
41+
extern void nr_php_show_exec(const char*, NR_EXECUTE_PROTO TSRMLS_DC);
42+
/* Limit length of execution context printed in the log file to 8 characters */
43+
#define NRP_SHOW_EXEC_CONTEXT(C) 8, NRSAFESTR(C)
44+
3045
/*
3146
* Purpose: Look through the PHP symbol table for special names or symbols
3247
* that provide additional hints that a specific framework has been loaded.

agent/php_globals.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,11 @@ typedef struct _nrphpglobals_t {
4949
* variable with the key `NEW_RELIC_LABELS` */
5050
#if ZEND_MODULE_API_NO >= ZEND_8_1_X_API_NO /* PHP 8.1+ */
5151
zend_long zend_offset; /* Zend extension offset */
52-
zend_long
53-
zend_op_array_offset; /* Zend extension op_array to modify reserved */
5452
#else
5553
int zend_offset; /* Zend extension offset */
56-
int zend_op_array_offset; /* Zend extension op_array to modify reserved */
54+
#endif
55+
#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP 8.0+ */
56+
int op_array_extension_handle; /* Zend op_array extension handle to attach agent's data to function */
5757
#endif
5858
int done_instrumentation; /* Set to true if we have installed instrumentation
5959
handlers */

agent/php_minit.c

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "php_internal_instrument.h"
2020
#include "php_samplers.h"
2121
#include "php_user_instrument.h"
22+
#include "php_user_instrument_wraprec_hashmap.h"
2223
#include "php_vm.h"
2324
#include "php_wrapper.h"
2425
#include "fw_laravel.h"
@@ -491,6 +492,16 @@ PHP_MINIT_FUNCTION(newrelic) {
491492
*/
492493
nr_php_generate_internal_wrap_records();
493494

495+
#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO
496+
/*
497+
* The user function wraprec hashmap must be initialized before INI processing
498+
* because INI processing adds wraprecs:
499+
* - newrelic.webtransaction.name.functions
500+
* - newrelic.transaction_tracer.custom
501+
*/
502+
nr_php_user_instrument_wraprec_hashmap_init();
503+
#endif
504+
494505
nr_php_register_ini_entries(module_number TSRMLS_CC);
495506

496507
if (0 == NR_PHP_PROCESS_GLOBALS(enabled)) {
@@ -720,8 +731,16 @@ PHP_MINIT_FUNCTION(newrelic) {
720731
nr_php_set_opcode_handlers();
721732

722733
nrl_debug(NRL_INIT, "MINIT processing done");
723-
#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP 7.4+ */
734+
#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP 8.0+ */
724735
NR_PHP_PROCESS_GLOBALS(zend_offset) = zend_get_resource_handle(dummy);
736+
NR_PHP_PROCESS_GLOBALS(op_array_extension_handle) = zend_get_op_array_extension_handle("newrelic");
737+
#if ZEND_MODULE_API_NO >= ZEND_8_4_X_API_NO /* PHP 8.4+ */
738+
/* When observer API is used by an extension, both handles (for user
739+
* and internal functions) must be initialized, even when one of them
740+
* is not used (as in our case). Observer API was changed in PHP 8.4.
741+
* For more details see: https://github.com/php/php-src/pull/14252 */
742+
(void) zend_get_internal_function_extension_handle("newrelic");
743+
#endif
725744
#else
726745
NR_PHP_PROCESS_GLOBALS(zend_offset) = zend_get_resource_handle(&dummy);
727746
#endif

0 commit comments

Comments
 (0)