-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Allow internal functions to declare if they support compile-time evaluation. #7780
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
If you're going to optimize this, please switch it to a prepopulated hash table. |
A nice solution might be to include this information as a flag in zend_func_info, which is generated from stubs. |
6e39a4f
to
add5428
Compare
bin2hex/hex2bin are done, strlen is already optimized to an opcode rather than a function call, and mb_strlen can't be done because it depends on ini settings that can change at runtime
preg_replace - I've avoided that because of https://www.regular-expressions.info/catastrophic.html potentially taking exponential time in the worst case, and bugs that have been seen in the past with PCRE's jit on different platforms, though maybe that isn't a problem in practice for compiling source code preg_match - same, also has a reference parameter
I'd considered doing that but didn't know if there were reasons that this wasn't already done - though, until the recent refactoring by @nikic it wouldn't even have been possible to catch notices/exceptions during compilation This would enable PECLs to start using this, which would be convenient if done properly. I also had a question on when it's safe to do this for floating-point math. Do we assume that rounding modes don't change (or does that not apply for
|
Done, the build is passing except for a spurious travis failure. This is ready for review.
Never mind, I see I was thinking of float-to-string conversion historically not being done in opcache until php 8.0, for https://wiki.php.net/rfc/locale_independent_float_to_string |
will this change have an relevant impact on how long files need to compile? |
It should be a tiny bit faster, I'd expect, with no longer having a long chain of string comparisons when compiling functions that have constant operands. PHP is already evaluating the most commonly used functions at compile-time. Apart from the new case-insensitive string functions (strtolower, strtoupper, strcasecmp, etc) any new functions added here are less commonly used. This also only affects compilation of calls that have constant operands - if the operands are not inferrable as constants at compile time then this code path doesn't get called.
If opcache.enable_cli isn't enabled or opcache isn't loaded, this has no impact, it isn't called. Also, in the interactive CLI ( |
@kocsismate fyi |
…uation. https://wiki.php.net/rfc/strtolower-ascii means that these functions no longer depend on the current locale in php 8.2. Before that, this was unsafe to evaluate at compile time. Followup to phpGH-7506 Add strcmp/strcasecmp/strtolower/strtoupper functions Add bin2hex/hex2bin and related functions Add array_key_first/array_key_last Update test of garbage collection using strtolower - Make sure that this tests the right thing when opcache is enabled. Update this as an example for future tests.
2b17d40
to
b6f17b4
Compare
Thanks for the ping (I already had a quick look at the PR before), and I liked it very much :) A question has just came to my mind though: wouldn't it make more sense to use a more general name for the annotation, like UPDATE: I've just realized that |
I agree, I'd avoided that name for that reason (functions may be too slow, use up too much memory, infinitely loop, etc). That name would also make the use of
|
This was accidentally added in phpGH-7780, but since it takes a callable argument, this flag is useless on this function. Closes phpGH-10859.
https://wiki.php.net/rfc/strtolower-ascii means that these functions no longer
depend on the current locale in php 8.2. Before that, this was unsafe to
evaluate at compile time.
Followup to GH-7506
Add strcmp/strcasecmp/strtolower/strtoupper functions
Add bin2hex/hex2bin and related functions
Add array_key_first/array_key_last
Update test of garbage collection using strtolower
Update this as an example for future tests.