Fix #75533: array_reduce is slow when $carry is large array

[crhg]

Bug #75533

result is no longer necessary after ZVAL_COPY(&args[0], &result);, but it keeps references to arrays while executing callback, causing unwanted copying when manipulating arrays in callback. This fix solves this problem by moving zval_ptr_dtor(&result); before zend_call_function.

test script (test_array_reduce.php):

<?php
$d = range(1, 20000);
$t0 = microtime(true);
$r = array_reduce(
        $d,
        function (&$carry, $item) {
            $carry []= $item;
            return $carry;
        },
        []
    );
printf("%fms\n", (microtime(true) - $t0) * 1000);

Without this fix:

% ./sapi/cli/php test_array_reduce.php
2870.778084ms

With this fix:

% ./sapi/cli/php test_array_reduce.php
9.871006ms

[krakjoe]

That doesn't look right ...

[motecshine]

may be result in some scenes dosent released

[crhg]

As long as zend_call_function for the callback function succeeds and doesn't return UNDEF, this PR simply changes the order of processing in the loop. Extracting only the part related to args[0] and result, it becomes as follows.

Without this diff:

    ZVAL_COPY(&args[0], &result);
    zend_call_function(&fci, &fci_cache)
    zval_ptr_dtor(&args[0]);
    zval_ptr_dtor(&result);
    ZVAL_COPY_VALUE(&result, &retval);

With this diff:

    ZVAL_COPY(&args[0], &result);
    zval_ptr_dtor(&result);
    zend_call_function(&fci, &fci_cache)
    zval_ptr_dtor(&args[0]);
    ZVAL_COPY_VALUE(&result, &retval);

At point before zval_ptr_dtor(&result); of new code, the reference count of the data referenced by result is at least 2, so it simply decrements the reference count by 1, and does not release the data.

The difference is that while executing zend_call_function on the callback function, the reference count of data is one less than that of the original code, thereby aiming at suppressing unwanted copying.

After zval_ptr_dtor(&args[0]);, it returns almost the same state as the original code.

If the callback function fails or returns undef, it enters the else clause. In this case the reference count to the data is one less than that of original code. Since result is not used, I do not think there is any particular problem, but I am not sure.

I would like to test about this case, I can not write a php script that goes to the else side. Please tell me if there is any good way.

[carusogabriel]

Both if and else branches have these zval_ptr_dtor(&args[1]); and zval_ptr_dtor(&args[0]); lines. Can't we move out of the check to avoid duplication?

[nikic]

Sorry for the delay, merged a slightly simplified form into 7.1+ via ab6c45f.