Disable JIT when pthread protection is unavailable#22853
Conversation
iluuu1994
left a comment
There was a problem hiding this comment.
Thank you Florian! Looks correct to me. The PR is still marked as draft, let me know when you think it's ok to merge.
| ===DONE=== | ||
| --EXPECTREGEX-- | ||
| ^(?:Warning: Cannot change opcache\.jit setting at run-time \(JIT is disabled\) in .+ on line 2 | ||
| )?===DONE===$ |
There was a problem hiding this comment.
I don't quite understand why this is a new problem? 🤔
Edit: Oh, I guess the JIT was just fully disabled at compile time and the INI setting didn't do anything.
There was a problem hiding this comment.
Exactly. After #22712 this behavior changed, but CI did not expose this particular follow-up problem because the initial test step printed a fatal error and failed. Consequently, the job never reached the later tracing/function JIT stages. See https://github.com/php/php-src/actions/runs/29793770728/job/88520997977
So the problem was already there with #22712, we just did not see it because CI failed before those stages were executed. 😉
There was a problem hiding this comment.
I am also unsure whether this warning represents a practical user-facing problem. It can occur when the configure-time check confirms that the pthread_jit_write_protect_np() symbol is available, but the runtime capability probe pthread_jit_write_protect_supported_np() returns false.
So far, I have only observed this on GitHub’s virtualized macOS runners. On a physical M4 Mac, the runtime capability is available and JIT works as expected.
|
Okay, given |
|
Ok for me |
|
Commited the revert in d84ae5b |
|
Thanks, but I need to commit this to 8.2+ separately anyway. |
|
I reverted the change directly. I believe, once you rebase, the changes of the test files directly in |
d84ae5b to
09aaa34
Compare
Avoid regex sections by conditionally enabling JIT.
iluuu1994
left a comment
There was a problem hiding this comment.
Thank you Florian! Will merge once CI has finished.
| <?php | ||
| // Skip when JIT was completely disabled at runtime. | ||
| if (ini_get('opcache.jit') !== 'disable') { | ||
| if (($status = opcache_get_status()) === false || $status['jit']['enabled']) { |
There was a problem hiding this comment.
Shouldn't this be:
if (($status = opcache_get_status()) !== false && $status['jit']['enabled']) {
? I guess it's fine to try to set it even with opcache disabled.
|
Thanks again! |
Follow up to #22712 (comment)
This moves the runtime capability check into
zend_jit_check_support(). Whenpthread_jit_write_protect_supported_np()reports that per-thread JIT write protection is unavailable, OPcache now logs anACCEL_LOG_WARNING, disables JIT, and continues using the interpreter. This matches the existing unsupported-JIT behavior.JIT tests that modify
opcache.jitat runtime now do so only when OPcache is inactive or its runtime status reports JIT as enabled. This preserves their original coverage on supported platforms without attempting to re-enable a JIT that the platform capability check disabled.Testing
TestandTest OpCacheconfigurations: 928 tests each, 0 failures.