Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:37343
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain realplain.com from 209.151.69.1 cause and error)
Message-ID: <00a701c8aaa9$e0fcd4c0$0201a8c0@pc1>
To: "Dmitry Stogov" <dmitry@zend.com>
Cc: <internals@lists.php.net>,
	"Andi Gutmans" <andi@zend.com>
References: <014b01c8a6db$57d53ba0$0201a8c0@pc1> <48181C02.3050705@zend.com>
Date: Wed, 30 Apr 2008 05:06:35 -0500
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [PATCH] More array filling optimizations
From: php_lists@realplain.com ("Matt Wilmas")

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
> >
> >