Allow some array functions to operate in-place using a peep-hole optimization

[nielsdos]

This patch optimizes the $x = array_function($x, ...) pattern and the
$x = array_function(temporary, ...) pattern for these functions:
array_merge, array_unique, array_replace, and array_intersect.
With these limitations:

• array_{unique,intersect} only do the temporary optimization because the
  comparison may throw.
• array_{merge,replace} only optimizes CVs for non-recursive case because the
  recursive version may throw.
• array_merge optimization works only if the array is packed and is
  without holes.

It works by checking if the array function is immediately followed by an
assignment which overrides the input. In that case we can do the
operation in-place instead of copying the array. Note that this is
limited to CV's at the moment, and can't handle more complex scenarios
like array or object assignments.
For the temporary case it suffices to check if the refcount of the input
array is 1 and the array is non-persistent and non-immutable.

The current approach is a bit ugly though: it looks at the VM
instructions from within a function to check if the optimization is
possible, which is a bit odd.
I considered extending opcache as an alternative, but I believe this would
require adding a whole bunch of machinery for only a few users.

Looking at the assembly of prepare_in_place_array_modify_if_possible()
it looks pretty light-weight, about 95 bytes / 29 instructions on my
x86-64 Linux laptop.

Safety

There are some array functions which take some sort of copy of the input
array into a temporary C array for sorting.
(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.

Results

Using this benchmark script
https://gist.github.com/nielsdos/ae5a2dddc53c61749ae31c908aa78e98 I get:

=== array_merge $a = array_merge($a, ...) ===
before 4.3821 sec
after 0.0022 sec
=== array_merge temporary ===
before 0.1265 sec
after 0.0479 sec
=== array_unique temporary ===
before 0.9297 sec
after 0.8498 sec
=== array_replace $a = array_replace($a, ...) ===
before 0.0810 sec
after 0.0083 sec
=== array_replace temporary ===
before 0.1261 sec
after 0.0534 sec
=== array_intersect temporary (no significant improvement because dominated by sorting) ===
before 30.499 sec
after 30.356 sec

[Girgias]

Technically, any comparison involving objects can throw, see: https://3v4l.org/64TZU

[nielsdos]

Thanks @Girgias for taking a look.
Tim made me aware of the error behaviour just recently. That is why there is a check that checks whether there is a user error handler to disallow the "input=output" optimization for array_unique.

In fact there are two optimizations

• RC1 optimization: if there is only one reference to an array, it's because it was passed as an argument and therefore it can be replaced. Even if there is an exception, a partial or in-place modification cannot be observed because the assignment never happened and the return value is destroyed.
• input=output optimization: if the input gets overwritten by the output, it is safe to do so if there are no exception handlers or other user code that can run in between the function call and the assignment.

The four functions: array_merge, array_unique, array_replace, and array_intersect all perform the RC1 optimization with this patch.

array_unique only performs the input=output optimization if the sorting algorithm is SORT_REGULAR or SORT_NUMERIC and no user error handler is installed.
array_merge and array_replace can do the input=output optimization safely in the non-recursive case.

Do you think there is something I'm overlooking?

[Girgias]

Thanks @Girgias for taking a look. Tim made me aware of the error behaviour just recently. That is why there is a check that checks whether there is a user error handler to disallow the "input=output" optimization for array_unique.

In fact there are two optimizations

* RC1 optimization: if there is only one reference to an array, it's because it was passed as an argument and therefore it can be replaced. Even if there is an exception, a partial or in-place modification cannot be observed because the assignment never happened and the return value is destroyed.

* input=output optimization: if the input gets overwritten by the output, it is safe to do so if there are no exception handlers or other user code that can run in between the function call and the assignment.

The four functions: array_merge, array_unique, array_replace, and array_intersect all perform the RC1 optimization with this patch.

array_unique only performs the input=output optimization if the sorting algorithm is SORT_REGULAR or SORT_NUMERIC and no user error handler is installed. array_merge and array_replace can do the input=output optimization safely in the non-recursive case.

Do you think there is something I'm overlooking?

So any internal object can define its own comparison handler, and thus can throw. If I modify the previous example to:

$o = gmp_init(10);
$fruits = array($o, []);
sort($fruits);

The call to sort() will throw a TypeError:

Fatal error: Uncaught TypeError: Number must be of type GMP|string|int, array given in /tmp/preview:5

So I'm not sure you can even safely assume that comparisons are not going to throw an exception even for SORT_NUMERIC and SORT_REGULAR, except if no objects are present, but that means traversing the array :/ (or us having typed arrays)

EDIT: arguably GMP's behaviour is slightly strange and needs fixing but we don't currently enforce extensions to not throw exceptions or emit notices (when we probably should) in the compare hook (nor the do_operation one)

[nielsdos]

I see. So that's indeed a problem for SORT_REGULAR. And it looks like there's a related problem for SORT_NUMERIC: the object->cast() method can throw...
That means I'll have to ban the optimization for array_unique, except for the RC1 case. Bummer, but it is what it is...

As always thanks for your insight.