From 078a286f6c555dd0edeeb6ab291dbb0be35b4a91 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 29 Mar 2025 22:40:02 +0100 Subject: [PATCH] Optimize SplFixedArray dimension performance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch optimizes reading and writing from SplFixedArray with the dimension operators. It accomplishes this due to the following optimizations: * Fast-path for long keys (inlined). * Optimization hints (UNEXPECTED + assertion) * Using an unsigned index so we can do a single length comparison For the following script: ```php $test = new SplFixedArray(4); for ($i = 0 ; $i< 5000000; $i++) $test[1] += $i; ``` On an i7-4790: ``` Benchmark 1: ./sapi/cli/php x.php Time (mean ± σ): 95.4 ms ± 1.6 ms [User: 91.5 ms, System: 3.2 ms] Range (min … max): 93.7 ms … 100.8 ms 31 runs Benchmark 2: ./sapi/cli/php_old x.php Time (mean ± σ): 119.1 ms ± 1.3 ms [User: 114.7 ms, System: 3.6 ms] Range (min … max): 117.6 ms … 123.1 ms 24 runs Summary ./sapi/cli/php x.php ran 1.25 ± 0.03 times faster than ./sapi/cli/php_old x.php ``` On an i7-1185G7: ``` Benchmark 1: ./sapi/cli/php x.php Time (mean ± σ): 67.9 ms ± 1.1 ms [User: 64.8 ms, System: 3.2 ms] Range (min … max): 66.6 ms … 72.8 ms 43 runs Benchmark 2: ./sapi/cli/php_old x.php Time (mean ± σ): 84.8 ms ± 1.1 ms [User: 81.0 ms, System: 3.9 ms] Range (min … max): 82.6 ms … 88.0 ms 34 runs Summary ./sapi/cli/php x.php ran 1.25 ± 0.03 times faster than ./sapi/cli/php_old x.php ``` --- ext/spl/spl_fixedarray.c | 52 +++++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/ext/spl/spl_fixedarray.c b/ext/spl/spl_fixedarray.c index b919501c0dd25..fa846e48eee03 100644 --- a/ext/spl/spl_fixedarray.c +++ b/ext/spl/spl_fixedarray.c @@ -324,14 +324,14 @@ static zend_object *spl_fixedarray_object_clone(zend_object *old_object) return new_object; } -static zend_long spl_offset_convert_to_long(zval *offset) /* {{{ */ +static zend_never_inline zend_ulong spl_offset_convert_to_ulong_slow(const zval *offset) /* {{{ */ { try_again: switch (Z_TYPE_P(offset)) { case IS_STRING: { zend_ulong index; if (ZEND_HANDLE_NUMERIC(Z_STR_P(offset), index)) { - return (zend_long) index; + return index; } break; } @@ -356,10 +356,22 @@ static zend_long spl_offset_convert_to_long(zval *offset) /* {{{ */ return 0; } -static zval *spl_fixedarray_object_read_dimension_helper(spl_fixedarray_object *intern, zval *offset) +/* Returned index is an unsigned number such that we don't have to do a negative check. + * Negative numbers will be mapped at indices larger than ZEND_ULONG_MAX, + * which is beyond the maximum length. */ +static zend_always_inline zend_ulong spl_offset_convert_to_ulong(const zval *offset) { - zend_long index; + if (EXPECTED(Z_TYPE_P(offset) == IS_LONG)) { + /* Allow skipping exception check at call-site. */ + ZEND_ASSERT(!EG(exception)); + return Z_LVAL_P(offset); + } else { + return spl_offset_convert_to_ulong_slow(offset); + } +} +static zval *spl_fixedarray_object_read_dimension_helper(spl_fixedarray_object *intern, zval *offset) +{ /* we have to return NULL on error here to avoid memleak because of * ZE duplicating uninitialized_zval_ptr */ if (!offset) { @@ -367,12 +379,12 @@ static zval *spl_fixedarray_object_read_dimension_helper(spl_fixedarray_object * return NULL; } - index = spl_offset_convert_to_long(offset); - if (EG(exception)) { + zend_ulong index = spl_offset_convert_to_ulong(offset); + if (UNEXPECTED(EG(exception))) { return NULL; } - if (index < 0 || index >= intern->array.size) { + if (UNEXPECTED(index >= intern->array.size)) { zend_throw_exception(spl_ce_OutOfBoundsException, "Index invalid or out of range", 0); return NULL; } else { @@ -407,22 +419,19 @@ static zval *spl_fixedarray_object_read_dimension(zend_object *object, zval *off static void spl_fixedarray_object_write_dimension_helper(spl_fixedarray_object *intern, zval *offset, zval *value) { - zend_long index; - if (!offset) { /* '$array[] = value' syntax is not supported */ zend_throw_error(NULL, "[] operator not supported for SplFixedArray"); return; } - index = spl_offset_convert_to_long(offset); - if (EG(exception)) { + zend_ulong index = spl_offset_convert_to_ulong(offset); + if (UNEXPECTED(EG(exception))) { return; } - if (index < 0 || index >= intern->array.size) { + if (UNEXPECTED(index >= intern->array.size)) { zend_throw_exception(spl_ce_OutOfBoundsException, "Index invalid or out of range", 0); - return; } else { /* Fix #81429 */ zval *ptr = &(intern->array.elements[index]); @@ -452,16 +461,13 @@ static void spl_fixedarray_object_write_dimension(zend_object *object, zval *off static void spl_fixedarray_object_unset_dimension_helper(spl_fixedarray_object *intern, zval *offset) { - zend_long index; - - index = spl_offset_convert_to_long(offset); - if (EG(exception)) { + zend_ulong index = spl_offset_convert_to_ulong(offset); + if (UNEXPECTED(EG(exception))) { return; } - if (index < 0 || index >= intern->array.size) { + if (UNEXPECTED(index >= intern->array.size)) { zend_throw_exception(spl_ce_OutOfBoundsException, "Index invalid or out of range", 0); - return; } else { zval garbage; ZVAL_COPY_VALUE(&garbage, &intern->array.elements[index]); @@ -483,14 +489,12 @@ static void spl_fixedarray_object_unset_dimension(zend_object *object, zval *off static bool spl_fixedarray_object_has_dimension_helper(spl_fixedarray_object *intern, zval *offset, bool check_empty) { - zend_long index; - - index = spl_offset_convert_to_long(offset); - if (EG(exception)) { + zend_ulong index = spl_offset_convert_to_ulong(offset); + if (UNEXPECTED(EG(exception))) { return false; } - if (index < 0 || index >= intern->array.size) { + if (index >= intern->array.size) { return false; }