diff --git a/UPGRADING b/UPGRADING index 1bba53274c16..65c45069c9bb 100644 --- a/UPGRADING +++ b/UPGRADING @@ -392,6 +392,8 @@ PHP 8.6 UPGRADE NOTES . Improved performance of array_map() with multiple arrays passed. . Improved performance of array_sum() and array_product() for integer-only arrays. + . Improved performance of min() and max() for integer-only and float-only + arrays. . Improved performance of array_unshift(). . Improved performance of array_walk(). . Improved performance of intval('+0b...', 2) and intval('0b...', 2). diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index b17922f84c3f..d9191dae5b28 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -35,6 +35,8 @@ PHP 8.6 INTERNALS UPGRADE NOTES . The zval_dtor() alias of zval_ptr_dtor_nogc() has been removed. Call zval_ptr_dtor_nogc() directly instead. . The internal zend_copy_parameters_array() function is no longer exposed. + . The internal zend_hash_minmax() function is no longer exposed. Scan the + HashTable directly and use zend_compare() for value comparisons instead. . The zend_make_callable() function has been removed, if a callable zval needs to be obtained use the zend_get_callable_zval_from_fcc() function instead. If this was used to store a callable, then an FCC should be diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index 2913af9c2b5f..acc342bc267d 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -3238,73 +3238,6 @@ ZEND_API int zend_hash_compare(HashTable *ht1, const HashTable *ht2, compare_fun } -ZEND_API zval* ZEND_FASTCALL zend_hash_minmax(const HashTable *ht, compare_func_t compar, uint32_t flag) -{ - uint32_t idx; - zval *res; - - IS_CONSISTENT(ht); - - if (ht->nNumOfElements == 0 ) { - return NULL; - } - - if (HT_IS_PACKED(ht)) { - zval *zv; - - idx = 0; - while (1) { - if (idx == ht->nNumUsed) { - return NULL; - } - if (Z_TYPE(ht->arPacked[idx]) != IS_UNDEF) break; - idx++; - } - res = ht->arPacked + idx; - for (; idx < ht->nNumUsed; idx++) { - zv = ht->arPacked + idx; - if (UNEXPECTED(Z_TYPE_P(zv) == IS_UNDEF)) continue; - - if (flag) { - if (compar(res, zv) < 0) { /* max */ - res = zv; - } - } else { - if (compar(res, zv) > 0) { /* min */ - res = zv; - } - } - } - } else { - Bucket *p; - - idx = 0; - while (1) { - if (idx == ht->nNumUsed) { - return NULL; - } - if (Z_TYPE(ht->arData[idx].val) != IS_UNDEF) break; - idx++; - } - res = &ht->arData[idx].val; - for (; idx < ht->nNumUsed; idx++) { - p = ht->arData + idx; - if (UNEXPECTED(Z_TYPE(p->val) == IS_UNDEF)) continue; - - if (flag) { - if (compar(res, &p->val) < 0) { /* max */ - res = &p->val; - } - } else { - if (compar(res, &p->val) > 0) { /* min */ - res = &p->val; - } - } - } - } - return res; -} - ZEND_API bool ZEND_FASTCALL _zend_handle_numeric_str_ex(const char *key, size_t length, zend_ulong *idx) { const char *tmp = key; diff --git a/Zend/zend_hash.h b/Zend/zend_hash.h index 6a695aeef449..a7790ee9e63b 100644 --- a/Zend/zend_hash.h +++ b/Zend/zend_hash.h @@ -303,8 +303,6 @@ typedef int (*bucket_compare_func_t)(Bucket *a, Bucket *b); ZEND_API int zend_hash_compare(HashTable *ht1, const HashTable *ht2, compare_func_t compar, bool ordered); ZEND_API void ZEND_FASTCALL zend_hash_sort_ex(HashTable *ht, sort_func_t sort_func, bucket_compare_func_t compare_func, bool renumber); ZEND_API void ZEND_FASTCALL zend_array_sort_ex(HashTable *ht, sort_func_t sort_func, bucket_compare_func_t compare_func, bool renumber); -ZEND_API zval* ZEND_FASTCALL zend_hash_minmax(const HashTable *ht, compare_func_t compar, uint32_t flag); - static zend_always_inline void ZEND_FASTCALL zend_hash_sort(HashTable *ht, bucket_compare_func_t compare_func, bool renumber) { zend_hash_sort_ex(ht, zend_sort, compare_func, renumber); } diff --git a/ext/standard/array.c b/ext/standard/array.c index 25259c47d61b..e4324275e82a 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -1089,9 +1089,151 @@ PHP_FUNCTION(key) } /* }}} */ -static int php_data_compare(const void *f, const void *s) /* {{{ */ +static zval *php_array_data_minmax(const HashTable *array, bool max) /* {{{ */ { - return zend_compare((zval*)f, (zval*)s); + uint32_t idx; + zval *res; + + if (array->nNumOfElements == 0) { + return NULL; + } + + if (HT_IS_PACKED(array)) { + zval *zv; + + idx = 0; + while (1) { + if (idx == array->nNumUsed) { + return NULL; + } + if (Z_TYPE(array->arPacked[idx]) != IS_UNDEF) { + break; + } + idx++; + } + res = array->arPacked + idx; + for (; idx < array->nNumUsed; idx++) { + zv = array->arPacked + idx; + if (UNEXPECTED(Z_TYPE_P(zv) == IS_UNDEF)) { + continue; + } + + if (max) { + if (zend_compare(res, zv) < 0) { + res = zv; + } + } else { + if (zend_compare(res, zv) > 0) { + res = zv; + } + } + } + } else { + Bucket *p; + + idx = 0; + while (1) { + if (idx == array->nNumUsed) { + return NULL; + } + if (Z_TYPE(array->arData[idx].val) != IS_UNDEF) { + break; + } + idx++; + } + res = &array->arData[idx].val; + for (; idx < array->nNumUsed; idx++) { + p = array->arData + idx; + if (UNEXPECTED(Z_TYPE(p->val) == IS_UNDEF)) { + continue; + } + + if (max) { + if (zend_compare(res, &p->val) < 0) { + res = &p->val; + } + } else { + if (zend_compare(res, &p->val) > 0) { + res = &p->val; + } + } + } + } + return res; +} +/* }}} */ + +static zend_always_inline bool php_array_minmax(HashTable *array, zval *return_value, bool max) /* {{{ */ +{ + zval *entry, *result = NULL; + zend_long result_lval = 0; + double result_dval = 0.0; + /* IS_LONG or IS_DOUBLE while every value scanned so far has that exact type, + * IS_UNDEF once a value forces the generic zend_compare() fallback. */ + uint8_t fast_type = IS_UNDEF; + + ZEND_HASH_FOREACH_VAL(array, entry) { + zval *value = entry; + + ZVAL_DEREF(value); + + if (fast_type == IS_LONG && EXPECTED(Z_TYPE_P(value) == IS_LONG)) { + zend_long value_lval = Z_LVAL_P(value); + if (max ? result_lval < value_lval : result_lval > value_lval) { + result = value; + result_lval = value_lval; + } + continue; + } + + if (fast_type == IS_DOUBLE && EXPECTED(Z_TYPE_P(value) == IS_DOUBLE)) { + double value_dval = Z_DVAL_P(value); + /* NaN ordering differs from zend_compare(); hand it to the fallback. */ + if (EXPECTED(!zend_isnan(value_dval))) { + if (max ? result_dval < value_dval : result_dval > value_dval) { + result = value; + result_dval = value_dval; + } + continue; + } + } + + if (!result) { + if (Z_TYPE_P(value) == IS_LONG) { + result = value; + result_lval = Z_LVAL_P(value); + fast_type = IS_LONG; + continue; + } + if (Z_TYPE_P(value) == IS_DOUBLE && !zend_isnan(Z_DVAL_P(value))) { + result = value; + result_dval = Z_DVAL_P(value); + fast_type = IS_DOUBLE; + continue; + } + return false; + } + + fast_type = IS_UNDEF; + + int cmp = zend_compare(result, value); + if (max ? cmp < 0 : cmp > 0) { + result = value; + } + } ZEND_HASH_FOREACH_END(); + + if (!result) { + return false; + } + + if (fast_type == IS_LONG) { + ZVAL_LONG(return_value, result_lval); + } else if (fast_type == IS_DOUBLE) { + ZVAL_DOUBLE(return_value, result_dval); + } else { + ZVAL_COPY_DEREF(return_value, result); + } + return true; } /* }}} */ @@ -1114,7 +1256,12 @@ PHP_FUNCTION(min) zend_argument_type_error(1, "must be of type array, %s given", zend_zval_value_name(&args[0])); RETURN_THROWS(); } else { - zval *result = zend_hash_minmax(Z_ARRVAL(args[0]), php_data_compare, 0); + HashTable *array = Z_ARRVAL(args[0]); + if (php_array_minmax(array, return_value, false)) { + return; + } + + zval *result = php_array_data_minmax(array, false); if (result) { RETURN_COPY_DEREF(result); } else { @@ -1242,7 +1389,12 @@ PHP_FUNCTION(max) zend_argument_type_error(1, "must be of type array, %s given", zend_zval_value_name(&args[0])); RETURN_THROWS(); } else { - zval *result = zend_hash_minmax(Z_ARRVAL(args[0]), php_data_compare, 1); + HashTable *array = Z_ARRVAL(args[0]); + if (php_array_minmax(array, return_value, true)) { + return; + } + + zval *result = php_array_data_minmax(array, true); if (result) { RETURN_COPY_DEREF(result); } else { diff --git a/ext/standard/tests/array/min_max_array_fast_path.phpt b/ext/standard/tests/array/min_max_array_fast_path.phpt new file mode 100644 index 000000000000..c9b613911c55 --- /dev/null +++ b/ext/standard/tests/array/min_max_array_fast_path.phpt @@ -0,0 +1,107 @@ +--TEST-- +min() and max() array long/double fast path preserves comparison behavior +--FILE-- + $values) { + echo "-- $name --\n"; + var_dump(min($values)); + var_dump(max($values)); +} + +?> +--EXPECT-- +-- packed long -- +int(-3) +int(7) +-- sparse long -- +int(-2) +int(7) +-- long refs -- +int(3) +int(9) +-- packed double -- +float(-3.5) +float(7.5) +-- sparse double -- +float(-2.5) +float(7.5) +-- double refs -- +float(3.5) +float(9.5) +-- double equal -- +float(2.5) +float(2.5) +-- nan first -- +float(1) +float(NAN) +-- nan middle -- +float(1) +float(3) +-- nan last -- +float(NAN) +float(3) +-- inf -- +float(-INF) +float(INF) +-- single long -- +int(7) +int(7) +-- single double -- +float(7.5) +float(7.5) +-- first non-long -- +int(3) +string(1) "5" +-- fallback after longs -- +int(2) +int(5) +-- fallback after doubles -- +float(2.5) +float(5.5) +-- long then double -- +float(2.5) +int(9) +-- double then long -- +int(2) +int(9)