Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not enable SPX when JIT is enabled #215

Open
NoiseByNorthwest opened this issue May 9, 2023 · 3 comments
Open

Do not enable SPX when JIT is enabled #215

NoiseByNorthwest opened this issue May 9, 2023 · 3 comments

Comments

@NoiseByNorthwest
Copy link
Owner

I've observed that when SPX is enabled, even if not active (i.e. auto_start=0), with JIT enabled (opcache.jit=tracing), it makes the execution flow somewhat corrupted (in my case an error is triggered with an inconsistent stack trace).

@NoiseByNorthwest
Copy link
Owner Author

The SPX enabling must be discarded in this case with a notice log. This incompatibility should also be mentionned in the README.

@ilnytskyi
Copy link
Contributor

Maybe native API for observation zend_observer_fcall_handlers and zend_observer_fcall_register can be added for PHP > 8 instead of overriding zend_execute_ex. The only disadvantage it does not capture internal functions calls, but only user defined, that is still covering most of the cases considering the fact profiling of builtins is disabled by default.
However I am not sure how this approach can affect metrics and other spx features compatibility.

xhprof doing something like this:

#if PHP_VERSION_ID >= 80000
#include "zend_observer.h"

https://github.com/tideways/php-xhprof-extension/blob/8b1571c76b8a1b68ac06cfce2861e44e2f6930fb/tideways_xhprof.c#L30

@NoiseByNorthwest
Copy link
Owner Author

Indeed, here is the corresponding patch thread tideways/php-xhprof-extension#96

I'm however wondering how it hurts performances and if the SPX hooking abstraction should in the end choose between a zend_execute_ex or a zend_observer based backend whether JIT is enabled or not. It would also allow to keep the internal function profiling feature.

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

No branches or pull requests

2 participants