Skip to content

Conversation

lavarou
Copy link
Member

@lavarou lavarou commented Apr 1, 2025

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.

lavarou added 7 commits April 1, 2025 12:50
The op_array extension slot for function is not available until
function's first call. Check if op_array extension slot is initialized
before using it to store wraprec.
Implement wraprec lookup by scope and function name. The implementation uses
two specialized hashmaps: one for global functions and another one for scopes.
The scopes hashmap's values are hashmaps for scoped methods. The hashmaps use
keys composed of not only the string and its length but also zend string's
hash. This hash is used to find the bucket index. The decision to include zend
string's hash was deliberate because the agent gets it for free when it needs
to lookup the wraprec based on scope and function name that are available in
execute_data's op array as zend_strings.
@newrelic-php-agent-bot
Copy link

newrelic-php-agent-bot commented Apr 1, 2025

Test Suite Status Result
Multiverse 8/8 passing
SOAK 85/85 passing

@lavarou lavarou changed the title refactor(agent): optimize user function instrumentation lookup refactor(agent): optimize user function instrumentation lookup [wip] Apr 1, 2025
lavarou added 9 commits April 1, 2025 14:14
When function is wrapped before its first use, its runtime cache is not yet
initialized - it gets initialized on first function execution. However, the
agent sometimes wraps functions before their execution. This type of use case
requires the agent to initialize function's runtime cache.
New method of storing named wraprecs requires the name of the method
to be wrapped to be in the case matching the code, i.e. wraprec name
matching is case sensitive.
Take advantage of observer API and new method of storing wraprecs and don't
install fcall_begin/fcall_end handlers for file execution - detect library/
framework and install wraprecs right in fcall_init!
Fix build for PHPs <= 7.4 - the name of variable pointing to linked list of
wraprecs for PHPs <= 7.4 is `nr_wrapped_user_functions`.
With new way of storing named wraprecs, agent's instrumentation no
longer depends on classes being loaded.
@lavarou lavarou force-pushed the refactor/optimize-wraprec-lookup branch from 6223b1a to 1ee6486 Compare April 1, 2025 18:14
@lavarou lavarou force-pushed the refactor/optimize-wraprec-lookup branch from bf28488 to 998970b Compare April 1, 2025 18:32
@codecov-commenter
Copy link

codecov-commenter commented Apr 1, 2025

Codecov Report

Attention: Patch coverage is 86.46154% with 44 lines in your changes missing coverage. Please review.

Project coverage is 77.26%. Comparing base (5a897c8) to head (952b2bc).

Files with missing lines Patch % Lines
agent/php_user_instrument.c 72.58% 17 Missing ⚠️
agent/php_user_instrument_wraprec_hashmap.c 93.50% 15 Missing ⚠️
agent/php_execute.c 14.28% 6 Missing ⚠️
agent/php_observer.c 62.50% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1042      +/-   ##
==========================================
+ Coverage   77.14%   77.26%   +0.11%     
==========================================
  Files         198      199       +1     
  Lines       27991    28285     +294     
==========================================
+ Hits        21593    21853     +260     
- Misses       6398     6432      +34     
Flag Coverage Δ
agent-for-php-7.2 77.60% <90.00%> (+0.14%) ⬆️
agent-for-php-7.3 77.61% <90.00%> (+0.14%) ⬆️
agent-for-php-7.4 77.33% <89.76%> (+0.14%) ⬆️
agent-for-php-8.0 76.54% <86.34%> (-0.05%) ⬇️
agent-for-php-8.1 76.85% <86.34%> (-0.05%) ⬇️
agent-for-php-8.2 76.47% <86.34%> (-0.05%) ⬇️
agent-for-php-8.3 76.47% <86.34%> (-0.05%) ⬇️
agent-for-php-8.4 76.49% <86.39%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Make it possible for wraprecs to be created when INI is processed. For that
to be possible, two things are necessary:
- the wraprec hashmap needs to be created before INI processing starts
- the pid stored in wraprec needs to be set using the value returned by
  nr_getpid because per request global pid (NRPRG(pid)) is not initialized
  until rinit.
`doRun` method is not defined in `Illuminate\Console\Application` scope.
It is defined in `Symfony\Component\Console\Application` scope.
lavarou added 2 commits May 5, 2025 15:11
The only way nr_php_user_instrument_wraprec_hashmap_init can fail is when
memory cannot be allocated, which is already considered a fatal error and
causes php process to exit so there's no point in returning failure status
in such case. Additionally, nrl_* (logging) does not work until INI has
been processed so the error message wouldn't get logged anyway.
@lavarou lavarou force-pushed the refactor/optimize-wraprec-lookup branch from 26ed257 to 8abf1eb Compare May 5, 2025 22:25
zsistla
zsistla previously approved these changes May 8, 2025
mfulb
mfulb previously approved these changes May 8, 2025
zsistla
zsistla previously approved these changes May 8, 2025
@lavarou lavarou dismissed stale reviews from zsistla and mfulb via 952b2bc May 8, 2025 15:26
@lavarou lavarou merged commit f6b2914 into dev May 8, 2025
65 checks passed
hahuja2 pushed a commit that referenced this pull request Jul 9, 2025
…Ps 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants