-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add opcache_preloading() internal function #19288
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know much about packing data structure declarations to reduce space, would it be better to put this next to the other bool fields above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hesitated, as I was weighting the benefits of packing vs placing the fields near related fields. I chose the latter because there is only one "instance" of this struct at a time, so this should not make a huge difference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
given that the relevant function is only being introduced in PHP 8.5, it doesn't make sense for zend_test_opcache_preloading() to exist on 8.4 or below, right? It would probably break if you tried to call it - suggest putting version guards around the declaration in the stub file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not be merged in 8.4, so the zend_test_opcache_preloading() function will not exist in 8.4 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, but it appears that zend_test supports running the new versions of the extension with older versions of PHP, since we generate arginfo to support older versions of PHP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If that's supported I don't think it's on purpose. To support old versions, adding guards in the stub wouldn't be enough, we would also need conditional directives in the implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Older versions of the arginfo are generated as a smoke test for the generator.
I wonder what the motivation is?
@nielsdos Similar motivation as here: #14479 (comment)
Uh oh!
There was an error while loading. Please reload this page.
Add a C function
opcache_preloading()that returnstrueduring preloading. Extensions can use this to detect preloading, not only during compilation/execution, but also inRINIT()/RSHUTDOWN().Since opcache currently doesn't install any header, I'm adding a new one:
zend_accelerator_api.h. Header name is based on other files inext/opcache.