For some reason "make test" with the patch reported several broken
array_splice() tests. Looking in gdb I saw that init_array() got -1 as a
size of new array.
I don't think checks for return_value_used for array_splice() have a lot
of sense.
Thanks. Dmitry.
Matt Wilmas wrote:
Hi Dmitry,
Yes, I noticed similar results with the compile/vm changes, and was also
thinking that it probably wasn't worth doing, so I just left it as a
separate patch in case someone else wanted to look into it. :-)
So I guess the rest is OK other than array_splice() tests breaking... I
didn't know the change to splice() would modify any behavior? I just added
a check "if (return_value_used)" to see whether or not to create and return
the array of removed elements -- like is done in next/prev/end/reset
functions. I thought it would only behave different (internally) when
array_splice() is used in a "void context":
array_splice(...); // Not used in assignment or function call
And the calling code wouldn't be affected? Maybe I don't get it, and that
check should be removed. :-)
Thanks for your feedback,
Matt
----- Original Message -----
From: "Dmitry Stogov"
Sent: Wednesday, April 30, 2008
Hi Matt,
I've made a review of your patch.
At first it has a bug at least in array_psplice() that makes several
tests to fail. So I removed the whole ext/standard part and tested only
Zend Engine changes.
Although the idea is interesting and implementation is perfect, the
patch doesn't make any speed improvement on real-life applications
(according to callgrind the patch can make 0.2% speedup, but callgrind
measure instruction fetches and not the speed itself) and showed small
slowdown on bench.php (the slowdown may be not the result of the patch
but result of worse code-locality caused by the patch).
I think we shouldn't make PHP more complicated without significant
result, so I suggest not to apply array_init_size_vm.diff part of the
patch.
Thanks. Dmitry.
Matt Wilmas wrote:
Hi all,
Expanding on the idea of passing a size other than 0 to
zend_hash_init(),
when possible, which was done awhile ago in a few areas (to save
resize/rehash operations), I finally added an "array_init_size()" that
can
be used instead of array_init(), likewise, when a size is known. As an
example to start, I updated these functions:
array_*: change_key_case, chunk, combine, fill, fill_keys, flip, keys
(without $search_value), map, rand, reverse, slice, splice, unique
(since it
copies all entries first), values; and also compact; func_get_args;
get_defined_vars; str_split; and a couple internal uses.
Those were the simplest, most obvious cases I found. :-) I didn't test
every function, but for example, the array functions that don't do much
work
I found to be ~10% faster (with more than 8 elements, otherwise no
resizing
would be needed). If these changes are applied, more can be done where
possible, by others, or I can and send further patches, or update them
myself if I have CVS access...
http://realplain.com/php/array_init_size.diff
http://realplain.com/php/array_init_size_5_3.diff
I was playing with initializing scripts' array( ... ) constructs with
the
correct size too, but wasn't sure about the changes or what the opinion
would be. Runtime creation is improved with >8 elements, though it adds
a
bit of compile overhead. Anyway, separate patches for that:
http://realplain.com/php/array_init_size_vm.diff
http://realplain.com/php/array_init_size_vm_5_3.diff
Thanks,
Matt
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
