Re: [PATCH] More array filling optimizations

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 > > > >