Re: Optimize zend_string equality check with hash key

On Fri, Apr 5, 2019 at 9:26 AM Benjamin Coutu <ben.coutu@zeyos.com> wrote: > Hello, > > I was surprised to find out that equality checks on zend_string do not > take advantage of the fact that in many cases we have a hash key available > that can be utilized in a quick bailout path without having to resort to a > costly memcmp on the value. > > I recommend to modify zend_string_equal_content like so: > > static zend_always_inline zend_bool zend_string_equal_content(zend_string > *s1, zend_string *s2) > { > if (ZSTR_LEN(s1) != ZSTR_LEN(s2)) { > return 0; > } > > if (ZSTR_H(s1) && ZSTR_H(s1) != ZSTR_H(s2)) { > return 0; > } > > return zend_string_equal_val(s1, s2); > } > > Thoughts? > We usually include the hash check explicitly in situations where it makes sense, see in particular uses of zend_string_equal_content inside zend_hash.c (such as https://github.com/php/php-src/blob/ade73c360cbe9d533214e8b7cb26c514c29256fb/Zend/zend_hash.c#L614). You'll note that these comparisons use the hash from the bucket rather than the string, for better locality. Including a hash check in zend_string_equal_content would result in a duplicate check there. I think it might make sense to include a hash check inside zend_string_equals() though, which is a higher-level function. From a quick look, only one use of this function performs it's own hash check, which could then be dropped. Nikita