Skip to content

Commit 1333407

Browse files
jmarblejmarble
committed
Refactor comparison function for better performance
Refactors php_array_data_compare_unstable_i() to dereference before type checking and simplify control flow with if/else-if chain. This eliminates redundant mode toggling and improves branch prediction. Pass transitive_mode parameter to compare_long_to_string() and compare_double_to_string() to reduce repeated EG() calls.
1 parent 8dca012 commit 1333407

File tree

2 files changed

+86
-65
lines changed

2 files changed

+86
-65
lines changed

‎Zend/zend_operators.c‎

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2257,7 +2257,7 @@ ZEND_API zend_result ZEND_FASTCALL compare_function(zval *result, zval *op1, zva
22572257
}
22582258
/* }}} */
22592259

2260-
static int compare_long_to_string(zend_long lval, zend_string *str) /* {{{ */
2260+
static int compare_long_to_string(zend_long lval, zend_string *str, bool transitive_mode) /* {{{ */
22612261
{
22622262
zend_long str_lval;
22632263
double str_dval;
@@ -2274,7 +2274,7 @@ static int compare_long_to_string(zend_long lval, zend_string *str) /* {{{ */
22742274
/* String is non-numeric. In transitive mode, enforce consistent ordering.
22752275
* Empty string < numeric < non-numeric string.
22762276
* Since str is non-numeric, check if it's empty. */
2277-
if (UNEXPECTED(EG(transitive_compare_mode))) {
2277+
if (UNEXPECTED(transitive_mode)) {
22782278
/* Empty string comes before everything */
22792279
if (ZSTR_LEN(str) == 0) {
22802280
return 1; /* lval > empty string */
@@ -2291,7 +2291,7 @@ static int compare_long_to_string(zend_long lval, zend_string *str) /* {{{ */
22912291
}
22922292
/* }}} */
22932293

2294-
static int compare_double_to_string(double dval, zend_string *str) /* {{{ */
2294+
static int compare_double_to_string(double dval, zend_string *str, bool transitive_mode) /* {{{ */
22952295
{
22962296
zend_long str_lval;
22972297
double str_dval;
@@ -2310,7 +2310,7 @@ static int compare_double_to_string(double dval, zend_string *str) /* {{{ */
23102310
/* String is non-numeric. In transitive mode, enforce consistent ordering.
23112311
* Empty string < numeric < non-numeric string.
23122312
* Since str is non-numeric, check if it's empty. */
2313-
if (UNEXPECTED(EG(transitive_compare_mode))) {
2313+
if (UNEXPECTED(transitive_mode)) {
23142314
/* Empty string comes before everything */
23152315
if (ZSTR_LEN(str) == 0) {
23162316
return 1; /* dval > empty string */
@@ -2331,6 +2331,8 @@ ZEND_API int ZEND_FASTCALL zend_compare(zval *op1, zval *op2) /* {{{ */
23312331
{
23322332
bool converted = false;
23332333
zval op1_copy, op2_copy;
2334+
2335+
bool transitive_mode = UNEXPECTED(EG(transitive_compare_mode));
23342336

23352337
while (1) {
23362338
switch (TYPE_PAIR(Z_TYPE_P(op1), Z_TYPE_P(op2))) {
@@ -2375,24 +2377,24 @@ ZEND_API int ZEND_FASTCALL zend_compare(zval *op1, zval *op2) /* {{{ */
23752377
return Z_STRLEN_P(op1) == 0 ? 0 : 1;
23762378

23772379
case TYPE_PAIR(IS_LONG, IS_STRING):
2378-
return compare_long_to_string(Z_LVAL_P(op1), Z_STR_P(op2));
2380+
return compare_long_to_string(Z_LVAL_P(op1), Z_STR_P(op2), transitive_mode);
23792381

23802382
case TYPE_PAIR(IS_STRING, IS_LONG):
2381-
return -compare_long_to_string(Z_LVAL_P(op2), Z_STR_P(op1));
2383+
return -compare_long_to_string(Z_LVAL_P(op2), Z_STR_P(op1), transitive_mode);
23822384

23832385
case TYPE_PAIR(IS_DOUBLE, IS_STRING):
23842386
if (zend_isnan(Z_DVAL_P(op1))) {
23852387
return 1;
23862388
}
23872389

2388-
return compare_double_to_string(Z_DVAL_P(op1), Z_STR_P(op2));
2390+
return compare_double_to_string(Z_DVAL_P(op1), Z_STR_P(op2), transitive_mode);
23892391

23902392
case TYPE_PAIR(IS_STRING, IS_DOUBLE):
23912393
if (zend_isnan(Z_DVAL_P(op2))) {
23922394
return 1;
23932395
}
23942396

2395-
return -compare_double_to_string(Z_DVAL_P(op2), Z_STR_P(op1));
2397+
return -compare_double_to_string(Z_DVAL_P(op2), Z_STR_P(op1), transitive_mode);
23962398

23972399
case TYPE_PAIR(IS_OBJECT, IS_NULL):
23982400
return 1;
@@ -3449,26 +3451,31 @@ ZEND_API int ZEND_FASTCALL zendi_smart_strcmp(zend_string *s1, zend_string *s2)
34493451
zend_long lval1 = 0, lval2 = 0;
34503452
double dval1 = 0.0, dval2 = 0.0;
34513453

3454+
/* Handle empty strings */
3455+
if (UNEXPECTED(s1->len == 0 || s2->len == 0)) {
3456+
if (UNEXPECTED(EG(transitive_compare_mode))) {
3457+
if (s1->len == 0 && s2->len == 0) return 0;
3458+
return s1->len == 0 ? -1 : 1;
3459+
}
3460+
goto string_cmp;
3461+
}
3462+
3463+
/* Skip numeric parsing if both strings start with letters */
3464+
unsigned char c1 = (unsigned char)s1->val[0];
3465+
unsigned char c2 = (unsigned char)s2->val[0];
3466+
3467+
if (((c1 >= 'a' && c1 <= 'z') || (c1 >= 'A' && c1 <= 'Z')) &&
3468+
((c2 >= 'a' && c2 <= 'z') || (c2 >= 'A' && c2 <= 'Z'))) {
3469+
goto string_cmp;
3470+
}
3471+
34523472
ret1 = is_numeric_string_ex(s1->val, s1->len, &lval1, &dval1, false, &oflow1, NULL);
34533473
ret2 = is_numeric_string_ex(s2->val, s2->len, &lval2, &dval2, false, &oflow2, NULL);
34543474

3455-
/* When in transitive comparison mode (used by SORT_REGULAR), enforce transitivity
3456-
* by consistently ordering numeric vs non-numeric strings. */
3475+
/* In transitive mode, enforce numeric < non-numeric ordering */
34573476
if (UNEXPECTED(EG(transitive_compare_mode))) {
34583477
int type_mismatch = (!!ret1) ^ (!!ret2);
34593478
if (UNEXPECTED(type_mismatch)) {
3460-
/* One is numeric, one is not.
3461-
* Special case: empty strings are non-numeric but sort BEFORE numeric strings.
3462-
* Order: empty < numeric < non-numeric (matches PHP 8+ comparison semantics) */
3463-
bool is_empty1 = (s1->len == 0);
3464-
bool is_empty2 = (s2->len == 0);
3465-
3466-
if (is_empty1 || is_empty2) {
3467-
/* If one is empty, empty comes first */
3468-
return is_empty1 ? -1 : 1;
3469-
}
3470-
3471-
/* Neither is empty: numeric < non-numeric */
34723479
return ret1 ? -1 : 1;
34733480
}
34743481
}

‎ext/standard/array.c‎

Lines changed: 57 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -304,62 +304,76 @@ static zend_always_inline int php_array_key_compare_string_locale_unstable_i(Buc
304304

305305
static zend_always_inline int php_array_data_compare_unstable_i(Bucket *f, Bucket *s) /* {{{ */
306306
{
307-
int result;
308-
309307
if (EXPECTED(Z_TYPE(f->val) == IS_LONG && Z_TYPE(s->val) == IS_LONG)) {
310-
zend_long l1 = Z_LVAL(f->val);
311-
zend_long l2 = Z_LVAL(s->val);
312-
return ZEND_THREEWAY_COMPARE(l1, l2);
308+
return ZEND_THREEWAY_COMPARE(Z_LVAL(f->val), Z_LVAL(s->val));
313309
}
314310

315-
if (Z_TYPE(f->val) == IS_STRING && Z_TYPE(s->val) == IS_STRING) {
316-
bool old_mode = EG(transitive_compare_mode);
317-
if (EXPECTED(!old_mode)) {
318-
EG(transitive_compare_mode) = true;
319-
result = zendi_smart_strcmp(Z_STR(f->val), Z_STR(s->val));
320-
EG(transitive_compare_mode) = false;
321-
} else {
322-
result = zendi_smart_strcmp(Z_STR(f->val), Z_STR(s->val));
323-
}
324-
goto check_enums;
325-
}
326-
327-
if (Z_TYPE(f->val) == IS_DOUBLE && Z_TYPE(s->val) == IS_DOUBLE) {
328-
double d1 = Z_DVAL(f->val);
329-
double d2 = Z_DVAL(s->val);
330-
return ZEND_THREEWAY_COMPARE(d1, d2);
311+
if (EXPECTED(Z_TYPE(f->val) == IS_DOUBLE && Z_TYPE(s->val) == IS_DOUBLE)) {
312+
return ZEND_THREEWAY_COMPARE(Z_DVAL(f->val), Z_DVAL(s->val));
331313
}
332314

333315
bool old_transitive_mode = EG(transitive_compare_mode);
334316
if (EXPECTED(!old_transitive_mode)) {
335317
EG(transitive_compare_mode) = true;
336-
result = zend_compare(&f->val, &s->val);
337-
EG(transitive_compare_mode) = false;
338-
} else {
339-
result = zend_compare(&f->val, &s->val);
340318
}
341319

342-
check_enums:
343-
;
344-
/* Special enums handling for array_unique. We don't want to add this logic to zend_compare as
345-
* that would be observable via comparison operators. */
346-
zval *rhs = &s->val;
347-
ZVAL_DEREF(rhs);
348-
if (UNEXPECTED(Z_TYPE_P(rhs) == IS_OBJECT)
349-
&& result == ZEND_UNCOMPARABLE
350-
&& (Z_OBJCE_P(rhs)->ce_flags & ZEND_ACC_ENUM)) {
351-
zval *lhs = &f->val;
352-
ZVAL_DEREF(lhs);
353-
if (Z_TYPE_P(lhs) == IS_OBJECT && (Z_OBJCE_P(lhs)->ce_flags & ZEND_ACC_ENUM)) {
354-
// Order doesn't matter, we just need to group the same enum values
355-
uintptr_t lhs_uintptr = (uintptr_t)Z_OBJ_P(lhs);
356-
uintptr_t rhs_uintptr = (uintptr_t)Z_OBJ_P(rhs);
357-
return lhs_uintptr == rhs_uintptr ? 0 : (lhs_uintptr < rhs_uintptr ? -1 : 1);
320+
int result;
321+
322+
/* Dereference before type checking */
323+
zval *op1 = &f->val;
324+
zval *op2 = &s->val;
325+
ZVAL_DEREF(op1);
326+
ZVAL_DEREF(op2);
327+
328+
if (Z_TYPE_P(op1) == IS_STRING && Z_TYPE_P(op2) == IS_STRING) {
329+
result = zendi_smart_strcmp(Z_STR_P(op1), Z_STR_P(op2));
330+
} else if (Z_TYPE_P(op1) == IS_OBJECT && Z_TYPE_P(op2) == IS_OBJECT) {
331+
if (Z_OBJ_P(op1) == Z_OBJ_P(op2)) {
332+
result = 0;
333+
} else if (Z_OBJCE_P(op1) != Z_OBJCE_P(op2)) {
334+
result = ZEND_UNCOMPARABLE;
335+
336+
/* Enum ordering for array_unique */
337+
if (UNEXPECTED((Z_OBJCE_P(op1)->ce_flags & ZEND_ACC_ENUM) ||
338+
(Z_OBJCE_P(op2)->ce_flags & ZEND_ACC_ENUM))) {
339+
if ((Z_OBJCE_P(op1)->ce_flags & ZEND_ACC_ENUM) &&
340+
!(Z_OBJCE_P(op2)->ce_flags & ZEND_ACC_ENUM)) {
341+
result = 1;
342+
} else if (!(Z_OBJCE_P(op1)->ce_flags & ZEND_ACC_ENUM) &&
343+
(Z_OBJCE_P(op2)->ce_flags & ZEND_ACC_ENUM)) {
344+
result = -1;
345+
} else {
346+
result = ZEND_THREEWAY_COMPARE((uintptr_t)Z_OBJ_P(op1), (uintptr_t)Z_OBJ_P(op2));
347+
}
348+
}
349+
} else if ((Z_OBJCE_P(op1)->ce_flags & ZEND_ACC_ENUM)) {
350+
result = ZEND_THREEWAY_COMPARE((uintptr_t)Z_OBJ_P(op1), (uintptr_t)Z_OBJ_P(op2));
358351
} else {
359-
// Shift enums to the end of the array
360-
return -1;
352+
result = zend_compare(op1, op2);
353+
}
354+
} else if (Z_TYPE_P(op1) == IS_ARRAY && Z_TYPE_P(op2) == IS_ARRAY) {
355+
if (Z_ARR_P(op1) == Z_ARR_P(op2)) {
356+
result = 0;
357+
} else {
358+
uint32_t n1 = zend_hash_num_elements(Z_ARRVAL_P(op1));
359+
uint32_t n2 = zend_hash_num_elements(Z_ARRVAL_P(op2));
360+
361+
if (n1 != n2) {
362+
/* Different sizes - order by size */
363+
result = ZEND_THREEWAY_COMPARE(n1, n2);
364+
} else {
365+
/* Same size - deep comparison */
366+
result = zend_compare(op1, op2);
367+
}
361368
}
369+
} else {
370+
result = zend_compare(op1, op2);
371+
}
372+
373+
if (EXPECTED(!old_transitive_mode)) {
374+
EG(transitive_compare_mode) = false;
362375
}
376+
363377
return result;
364378
}
365379
/* }}} */

0 commit comments

Comments
 (0)