Skip to content

Commit bb5deb3

Browse files
committed
Allow array functions to operate in-place if the refcount is 1
This allows array_merge, array_intersect, array_replace, array_unique and usort to avoid taking a copy and do the transformation in-place. ** Safety ** There are some array functions which take a copy of the input array into a temporary C array for sorting purposes. (e.g. array_unique, array_diff, and array_intersect do this). Since we no longer take a copy in all cases, we must check if it's possible that a value is accessed that was already destroyed. For array_unique: cmpdata will never be removed so that will never reach refcount 0. And when something is removed, it is the previous value of cmpdata, not the one user later. So this seems okay. For array_intersect: a previous pointer (ptr[0] - 1) is accessed. But this can't be a destroyed value because the pointer is first moved forward. For array_diff: it's possible a previous pointer is accessed after destruction. So we can't optimise this case easily.
1 parent 976d7ed commit bb5deb3

File tree

1 file changed

+80
-21
lines changed

1 file changed

+80
-21
lines changed

ext/standard/array.c

Lines changed: 80 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@ PHP_MSHUTDOWN_FUNCTION(array) /* {{{ */
9191
}
9292
/* }}} */
9393

94+
static zend_always_inline bool zend_may_modify_array_in_place(const zval *arg)
95+
{
96+
return !(GC_FLAGS(Z_ARRVAL_P(arg)) & (IS_ARRAY_IMMUTABLE | IS_ARRAY_PERSISTENT)) && Z_REFCOUNT_P(arg) == 1;
97+
}
98+
9499
static zend_never_inline ZEND_COLD int stable_sort_fallback(Bucket *a, Bucket *b) {
95100
if (Z_EXTRA(a->val) > Z_EXTRA(b->val)) {
96101
return 1;
@@ -901,11 +906,19 @@ static void php_usort(INTERNAL_FUNCTION_PARAMETERS, bucket_compare_func_t compar
901906
RETURN_TRUE;
902907
}
903908

904-
/* Copy array, so the in-place modifications will not be visible to the callback function */
905-
arr = zend_array_dup(arr);
909+
/* Copy array, so the in-place modifications will not be visible to the callback function.
910+
* Unless there are no other references since we know for sure it won't be visible. */
911+
bool in_place = zend_may_modify_array_in_place(array);
912+
if (!in_place) {
913+
arr = zend_array_dup(arr);
914+
}
906915

907916
zend_hash_sort(arr, compare_func, renumber);
908917

918+
if (in_place) {
919+
GC_ADDREF(arr);
920+
}
921+
909922
zval garbage;
910923
ZVAL_COPY_VALUE(&garbage, array);
911924
ZVAL_ARR(array, arr);
@@ -3866,10 +3879,17 @@ static zend_always_inline void php_array_replace_wrapper(INTERNAL_FUNCTION_PARAM
38663879
}
38673880
}
38683881

3869-
/* copy first array */
3882+
/* copy first array if necessary */
38703883
arg = args;
3871-
dest = zend_array_dup(Z_ARRVAL_P(arg));
3884+
bool in_place = zend_may_modify_array_in_place(arg);
3885+
if (in_place) {
3886+
dest = Z_ARRVAL_P(arg);
3887+
} else {
3888+
dest = zend_array_dup(Z_ARRVAL_P(arg));
3889+
}
3890+
38723891
ZVAL_ARR(return_value, dest);
3892+
38733893
if (recursive) {
38743894
for (i = 1; i < argc; i++) {
38753895
arg = args + i;
@@ -3881,6 +3901,10 @@ static zend_always_inline void php_array_replace_wrapper(INTERNAL_FUNCTION_PARAM
38813901
zend_hash_merge(dest, Z_ARRVAL_P(arg), zval_add_ref, 1);
38823902
}
38833903
}
3904+
3905+
if (in_place) {
3906+
GC_ADDREF(dest);
3907+
}
38843908
}
38853909
/* }}} */
38863910

@@ -3945,22 +3969,34 @@ static zend_always_inline void php_array_merge_wrapper(INTERNAL_FUNCTION_PARAMET
39453969

39463970
arg = args;
39473971
src = Z_ARRVAL_P(arg);
3948-
/* copy first array */
3949-
array_init_size(return_value, count);
3950-
dest = Z_ARRVAL_P(return_value);
3972+
/* copy first array if necessary */
3973+
bool in_place = false;
39513974
if (HT_IS_PACKED(src)) {
3952-
zend_hash_real_init_packed(dest);
3953-
ZEND_HASH_FILL_PACKED(dest) {
3954-
ZEND_HASH_PACKED_FOREACH_VAL(src, src_entry) {
3955-
if (UNEXPECTED(Z_ISREF_P(src_entry) &&
3956-
Z_REFCOUNT_P(src_entry) == 1)) {
3957-
src_entry = Z_REFVAL_P(src_entry);
3958-
}
3959-
Z_TRY_ADDREF_P(src_entry);
3960-
ZEND_HASH_FILL_ADD(src_entry);
3961-
} ZEND_HASH_FOREACH_END();
3962-
} ZEND_HASH_FILL_END();
3975+
/* Note: If it has holes, it might get sequentialized */
3976+
if (HT_IS_WITHOUT_HOLES(src) && zend_may_modify_array_in_place(arg)) {
3977+
dest = src;
3978+
in_place = true;
3979+
ZVAL_ARR(return_value, dest);
3980+
} else {
3981+
array_init_size(return_value, count);
3982+
dest = Z_ARRVAL_P(return_value);
3983+
3984+
zend_hash_real_init_packed(dest);
3985+
ZEND_HASH_FILL_PACKED(dest) {
3986+
ZEND_HASH_PACKED_FOREACH_VAL(src, src_entry) {
3987+
if (UNEXPECTED(Z_ISREF_P(src_entry) &&
3988+
Z_REFCOUNT_P(src_entry) == 1)) {
3989+
src_entry = Z_REFVAL_P(src_entry);
3990+
}
3991+
Z_TRY_ADDREF_P(src_entry);
3992+
ZEND_HASH_FILL_ADD(src_entry);
3993+
} ZEND_HASH_FOREACH_END();
3994+
} ZEND_HASH_FILL_END();
3995+
}
39633996
} else {
3997+
array_init_size(return_value, count);
3998+
dest = Z_ARRVAL_P(return_value);
3999+
39644000
zend_string *string_key;
39654001
zend_hash_real_init_mixed(dest);
39664002
ZEND_HASH_MAP_FOREACH_STR_KEY_VAL(src, string_key, src_entry) {
@@ -3987,6 +4023,10 @@ static zend_always_inline void php_array_merge_wrapper(INTERNAL_FUNCTION_PARAMET
39874023
php_array_merge(dest, Z_ARRVAL_P(arg));
39884024
}
39894025
}
4026+
4027+
if (in_place) {
4028+
GC_ADDREF(dest);
4029+
}
39904030
}
39914031
/* }}} */
39924032

@@ -4594,7 +4634,12 @@ PHP_FUNCTION(array_unique)
45944634

45954635
cmp = php_get_data_compare_func_unstable(sort_type, 0);
45964636

4597-
RETVAL_ARR(zend_array_dup(Z_ARRVAL_P(array)));
4637+
bool in_place = zend_may_modify_array_in_place(array);
4638+
if (in_place) {
4639+
RETVAL_ARR(Z_ARRVAL_P(array));
4640+
} else {
4641+
RETVAL_ARR(zend_array_dup(Z_ARRVAL_P(array)));
4642+
}
45984643

45994644
/* create and sort array with pointers to the target_hash buckets */
46004645
arTmp = pemalloc((Z_ARRVAL_P(array)->nNumOfElements + 1) * sizeof(struct bucketindex), GC_FLAGS(Z_ARRVAL_P(array)) & IS_ARRAY_PERSISTENT);
@@ -4640,6 +4685,10 @@ PHP_FUNCTION(array_unique)
46404685
}
46414686
}
46424687
pefree(arTmp, GC_FLAGS(Z_ARRVAL_P(array)) & IS_ARRAY_PERSISTENT);
4688+
4689+
if (in_place) {
4690+
Z_ADDREF_P(return_value);
4691+
}
46434692
}
46444693
/* }}} */
46454694

@@ -4764,6 +4813,7 @@ static void php_array_intersect(INTERNAL_FUNCTION_PARAMETERS, int behavior, int
47644813
zend_fcall_info *fci_key = NULL, *fci_data;
47654814
zend_fcall_info_cache *fci_key_cache = NULL, *fci_data_cache;
47664815
PHP_ARRAY_CMP_FUNC_VARS;
4816+
bool in_place = false;
47674817

47684818
bucket_compare_func_t intersect_key_compare_func;
47694819
bucket_compare_func_t intersect_data_compare_func;
@@ -4890,8 +4940,13 @@ static void php_array_intersect(INTERNAL_FUNCTION_PARAMETERS, int behavior, int
48904940
}
48914941
}
48924942

4893-
/* copy the argument array */
4894-
RETVAL_ARR(zend_array_dup(Z_ARRVAL(args[0])));
4943+
/* copy the argument array if necessary */
4944+
in_place = zend_may_modify_array_in_place(&args[0]);
4945+
if (in_place) {
4946+
RETVAL_ARR(Z_ARRVAL_P(&args[0]));
4947+
} else {
4948+
RETVAL_ARR(zend_array_dup(Z_ARRVAL_P(&args[0])));
4949+
}
48954950

48964951
/* go through the lists and look for common values */
48974952
while (Z_TYPE(ptrs[0]->val) != IS_UNDEF) {
@@ -5002,6 +5057,10 @@ static void php_array_intersect(INTERNAL_FUNCTION_PARAMETERS, int behavior, int
50025057

50035058
efree(ptrs);
50045059
efree(lists);
5060+
5061+
if (in_place) {
5062+
Z_ADDREF_P(return_value);
5063+
}
50055064
}
50065065
/* }}} */
50075066

0 commit comments

Comments
 (0)