Eligibility of array_key_exists() optimization for 7.3

php.internals

Michael Moravec

8 years ago
Hi, there are some questions whether PR #3360 (Implement ZEND_ARRAY_KEY_EXISTS opcode to speed up array_key_exists()) [1] could go into 7.3.0beta2 or not. This PR adds new opcode, but also significantly improves performance of array_key_exists(), to the point that it becomes faster than isset(). This function is often used in major OSS libraries and some of them use hacks with pre-checking using isset() to avoid the performance overhead of calling this PHP function. Language-wise it's similar to ZEND_COUNT or ZEND_IN_ARRAY - only a specific instruction with a VM handler to eliminate the function calling overhead. This new opcode may have some implications for extensions, though quickly grepping trough XDebug and PHPDBG source didn't reveal any specific handling for ZEND_COUNT or ZEND_IN_ARRAY. Do you think this optimization could still go into 7.3 so the PHP ecosystem could benefit from this improvement (possibly could be reverted in case of issues)? Or does it have to wait for 16 months? Thanks, M. [1] https://github.com/php/php-src/pull/3360

Sara Golemon

8 years ago
On Wed, Aug 8, 2018 at 12:11 PM, Michael Moravec <php.net@majkl578.cz> wrote:
> Do you think this optimization could still go into 7.3 so the PHP ecosystem > could benefit from this improvement (possibly could be reverted in case of > issues)? >
My vote is no. It's an optimization and optimizations are yay, but feature freeze isn't just a PR label, it's a release process milestone. The tradeoff against code-stability just isn't there. -Sara

Levi Morrison

8 years ago
On Wed, Aug 8, 2018 at 12:50 PM Sara Golemon <pollita@php.net> wrote:
> > On Wed, Aug 8, 2018 at 12:11 PM, Michael Moravec <php.net@majkl578.cz> wrote: > > Do you think this optimization could still go into 7.3 so the PHP ecosystem > > could benefit from this improvement (possibly could be reverted in case of > > issues)? > > > My vote is no. > > It's an optimization and optimizations are yay, but feature freeze > isn't just a PR label, it's a release process milestone. The tradeoff > against code-stability just isn't there. > > -Sara > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php
I agree with Sara. Sadly, I also have some similar improvements I wanted to get in for improving the performance of `array_slice` but couldn't meet feature freeze. If we were to bring in my changes and yours and who knows who else may have lurking changes like this then the stability risk Sara mentioned is real. It hurts to wait for another release but the bright side is that is a lot of time for the feature to get tested and be far less risky.