FAQ
Hi,

I'm proposing two simple patches that eliminate a lot of useless zval
copying.
For example they remove only about 800 calls to zend_hash_copy() (25%) on
each request to wordpress-3.6.0 home page and make it 2-4% faster.

It's not a questions about master branch, but I think it is also safe to
commit them into PHP-5.4 and PHP-5.5.

Any objections?

https://gist.github.com/dstogov/7117623

https://gist.github.com/dstogov/7117649

Thanks. Dmitry.

Search Discussions

  • Nikita Popov at Oct 23, 2013 at 1:52 pm

    On Wed, Oct 23, 2013 at 2:36 PM, Dmitry Stogov wrote:

    Hi,

    I'm proposing two simple patches that eliminate a lot of useless zval
    copying.
    For example they remove only about 800 calls to zend_hash_copy() (25%) on
    each request to wordpress-3.6.0 home page and make it 2-4% faster.

    It's not a questions about master branch, but I think it is also safe to
    commit them into PHP-5.4 and PHP-5.5.

    Any objections?

    https://gist.github.com/dstogov/7117623

    https://gist.github.com/dstogov/7117649

    Thanks. Dmitry.
    No objection, just quick note on func_get_args patch: You can use
    SEPARATE_ARG_IF_REF there, so the code in the loop body is:

         zval *arg = *(p-(arg_count-i));
         SEPARATE_ARG_IF_REF(arg);
         zend_hash_next_index_insert(Z_ARRVAL_P(return_value), &arg, sizeof(zval
    *), NULL);

    Nikita
  • Dmitry Stogov at Oct 23, 2013 at 2:00 pm
    makes sense. thanks :)

    Dmitry.

    On Wed, Oct 23, 2013 at 5:52 PM, Nikita Popov wrote:
    On Wed, Oct 23, 2013 at 2:36 PM, Dmitry Stogov wrote:

    Hi,

    I'm proposing two simple patches that eliminate a lot of useless zval
    copying.
    For example they remove only about 800 calls to zend_hash_copy() (25%) on
    each request to wordpress-3.6.0 home page and make it 2-4% faster.

    It's not a questions about master branch, but I think it is also safe to
    commit them into PHP-5.4 and PHP-5.5.

    Any objections?

    https://gist.github.com/dstogov/7117623

    https://gist.github.com/dstogov/7117649

    Thanks. Dmitry.
    No objection, just quick note on func_get_args patch: You can use
    SEPARATE_ARG_IF_REF there, so the code in the loop body is:

    zval *arg = *(p-(arg_count-i));
    SEPARATE_ARG_IF_REF(arg);
    zend_hash_next_index_insert(Z_ARRVAL_P(return_value), &arg,
    sizeof(zval *), NULL);

    Nikita
  • Levi Morrison at Oct 23, 2013 at 2:21 pm

    I'm proposing two simple patches that eliminate a lot of useless zval
    copying.
    For example they remove only about 800 calls to zend_hash_copy() (25%) on
    each request to wordpress-3.6.0 home page and make it 2-4% faster.
    Sounds great. Thank you for looking into this. Just to clarify: the
    wordpress improvements were from both patches, yes?

    It's not a questions about master branch, but I think it is also safe to
    commit them into PHP-5.4 and PHP-5.5.

    Any objections?
    I don't have an objection; I just want to reiterate that changing code can
    introduce BC breaks and to be careful when backporting it. Just be careful.

    Again, good work.
  • Dmitry Stogov at Oct 23, 2013 at 7:20 pm

    On Wed, Oct 23, 2013 at 6:21 PM, Levi Morrison wrote:
    I'm proposing two simple patches that eliminate a lot of useless zval
    copying.
    For example they remove only about 800 calls to zend_hash_copy() (25%) on
    each request to wordpress-3.6.0 home page and make it 2-4% faster.
    Sounds great. Thank you for looking into this. Just to clarify: the
    wordpress improvements were from both patches, yes?
    Yes, both patches together.

    It's not a questions about master branch, but I think it is also safe to
    commit them into PHP-5.4 and PHP-5.5.

    Any objections?
    I don't have an objection; I just want to reiterate that changing code can
    introduce BC breaks and to be careful when backporting it. Just be careful.
    Of course we checked them careful for possible BC breaks, but we may miss
    some special cases.

    Thanks. Dmitry.
  • Patrick ALLAERT at Oct 23, 2013 at 6:10 pm

    2013/10/23 Dmitry Stogov <dmitry@zend.com>:
    Hi,

    I'm proposing two simple patches that eliminate a lot of useless zval
    copying.
    For example they remove only about 800 calls to zend_hash_copy() (25%) on
    each request to wordpress-3.6.0 home page and make it 2-4% faster.

    It's not a questions about master branch, but I think it is also safe to
    commit them into PHP-5.4 and PHP-5.5.

    Any objections?

    https://gist.github.com/dstogov/7117623

    https://gist.github.com/dstogov/7117649

    Thanks. Dmitry.
    That sounds really good.

    Note that in many performance investigations I made, array_merge() was
    often one of the cause of some unnecessary overhead, however, most of
    the times this function was used was in fact to merge arrays that were
    in fact *hashes* (dictionary, map,...) and not really *arrays*. In
    those cases, the performance fix is simply to use "$y + $x" instead of
    "array_merge($x, $y).

    Are all those array_merge() of wordpress really made to merge *arrays*
    rather than *hashes*?

    Is the gain constant for all cases of array_merge(), with numeric
    indices vs string ones?

    Patrick
  • Dmitry Stogov at Oct 23, 2013 at 7:23 pm
    No. the array_merge() patch doesn't change the algorithm.
    We just discovered that array_merge() makes a copy of each element of each
    array before the actual merging, and in case it's an array or object, it
    may take significant time, but this copying is absolutely useless. The
    patch just remove it.

    Thanks. Dmitry.

    On Wed, Oct 23, 2013 at 10:10 PM, Patrick ALLAERT wrote:

    2013/10/23 Dmitry Stogov <dmitry@zend.com>:
    Hi,

    I'm proposing two simple patches that eliminate a lot of useless zval
    copying.
    For example they remove only about 800 calls to zend_hash_copy() (25%) on
    each request to wordpress-3.6.0 home page and make it 2-4% faster.

    It's not a questions about master branch, but I think it is also safe to
    commit them into PHP-5.4 and PHP-5.5.

    Any objections?

    https://gist.github.com/dstogov/7117623

    https://gist.github.com/dstogov/7117649

    Thanks. Dmitry.
    That sounds really good.

    Note that in many performance investigations I made, array_merge() was
    often one of the cause of some unnecessary overhead, however, most of
    the times this function was used was in fact to merge arrays that were
    in fact *hashes* (dictionary, map,...) and not really *arrays*. In
    those cases, the performance fix is simply to use "$y + $x" instead of
    "array_merge($x, $y).

    Are all those array_merge() of wordpress really made to merge *arrays*
    rather than *hashes*?

    Is the gain constant for all cases of array_merge(), with numeric
    indices vs string ones?

    Patrick
  • Christopher Jones at Oct 24, 2013 at 12:17 am

    On 10/23/2013 05:36 AM, Dmitry Stogov wrote:
    Hi,

    I'm proposing two simple patches that eliminate a lot of useless zval
    copying.
    For example they remove only about 800 calls to zend_hash_copy() (25%) on
    each request to wordpress-3.6.0 home page and make it 2-4% faster.

    It's not a questions about master branch, but I think it is also safe to
    commit them into PHP-5.4 and PHP-5.5.

    Any objections?

    https://gist.github.com/dstogov/7117623

    https://gist.github.com/dstogov/7117649

    Thanks. Dmitry.
    I'd prefer PHP 5.4 was kept stable.

    Last time I suggested keeping PHP 5.4 stable [1], it wasn't, and the result was
    a performance regression [2].

    Chris

    References:

    [1] http://marc.info/?l=php-internals&m=135820887521861

    [2] http://marc.info/?l=php-internals&m=137037681223946
  • Pierre Joye at Oct 24, 2013 at 5:25 am
    hi,

    On Thu, Oct 24, 2013 at 2:17 AM, Christopher Jones
    wrote:
    On 10/23/2013 05:36 AM, Dmitry Stogov wrote:

    Hi,

    I'm proposing two simple patches that eliminate a lot of useless zval
    copying.
    For example they remove only about 800 calls to zend_hash_copy() (25%) on
    each request to wordpress-3.6.0 home page and make it 2-4% faster.

    It's not a questions about master branch, but I think it is also safe to
    commit them into PHP-5.4 and PHP-5.5.

    Any objections?

    https://gist.github.com/dstogov/7117623

    https://gist.github.com/dstogov/7117649

    Thanks. Dmitry.
    I'd prefer PHP 5.4 was kept stable.

    Last time I suggested keeping PHP 5.4 stable [1], it wasn't, and the result
    was
    a performance regression [2].
    I tend to agree with Chris here, even for 5.5 :)

    However, Let me ask Matt to run our full tests suite against your
    patch, that may give us some numbers to discuss or confirm the safety
    of this patch.

    Matt? Can you run pftt, apps compat and perf tests using this patch pls?

    Cheers,
    --
    Pierre

    @pierrejoye | http://www.libgd.org
  • Pierre Joye at Oct 24, 2013 at 5:26 am
    fix Julien email
    On Thu, Oct 24, 2013 at 7:25 AM, Pierre Joye wrote:
    hi,

    On Thu, Oct 24, 2013 at 2:17 AM, Christopher Jones
    wrote:
    On 10/23/2013 05:36 AM, Dmitry Stogov wrote:

    Hi,

    I'm proposing two simple patches that eliminate a lot of useless zval
    copying.
    For example they remove only about 800 calls to zend_hash_copy() (25%) on
    each request to wordpress-3.6.0 home page and make it 2-4% faster.

    It's not a questions about master branch, but I think it is also safe to
    commit them into PHP-5.4 and PHP-5.5.

    Any objections?

    https://gist.github.com/dstogov/7117623

    https://gist.github.com/dstogov/7117649

    Thanks. Dmitry.
    I'd prefer PHP 5.4 was kept stable.

    Last time I suggested keeping PHP 5.4 stable [1], it wasn't, and the result
    was
    a performance regression [2].
    I tend to agree with Chris here, even for 5.5 :)

    However, Let me ask Matt to run our full tests suite against your
    patch, that may give us some numbers to discuss or confirm the safety
    of this patch.

    Matt? Can you run pftt, apps compat and perf tests using this patch pls?

    Cheers,
    --
    Pierre

    @pierrejoye | http://www.libgd.org


    --
    Pierre

    @pierrejoye | http://www.libgd.org
  • Dmitry Stogov at Oct 24, 2013 at 6:36 am

    On Thu, Oct 24, 2013 at 9:25 AM, Pierre Joye wrote:

    hi,

    On Thu, Oct 24, 2013 at 2:17 AM, Christopher Jones
    wrote:
    On 10/23/2013 05:36 AM, Dmitry Stogov wrote:

    Hi,

    I'm proposing two simple patches that eliminate a lot of useless zval
    copying.
    For example they remove only about 800 calls to zend_hash_copy() (25%)
    on
    each request to wordpress-3.6.0 home page and make it 2-4% faster.

    It's not a questions about master branch, but I think it is also safe to
    commit them into PHP-5.4 and PHP-5.5.

    Any objections?

    https://gist.github.com/dstogov/7117623

    https://gist.github.com/dstogov/7117649

    Thanks. Dmitry.
    I'd prefer PHP 5.4 was kept stable.

    Last time I suggested keeping PHP 5.4 stable [1], it wasn't, and the result
    was
    a performance regression [2].
    I tend to agree with Chris here, even for 5.5 :)
    I don't care a lot about 5.4, but think it should be fixed in 5.5.

    However, Let me ask Matt to run our full tests suite against your
    patch, that may give us some numbers to discuss or confirm the safety
    of this patch.
    Of course we checked patch with PHPT tests and some real-life apps, but in
    case you have any other test suites it would be a great help to run them
    too.

    I would also look into strtr() implementation that became 6 times slower on
    typical usage in comparison to 5.3 after its "improvement" :(

    <?php
    function foo() {
         for ($i = 0; $i < 100000; $i++) {
             strtr("abcdefgh", array("a"=>"11", "g"=>"22"));
         }
    }
    foo();
    ?>

    Thanks. Dmitry.

    Matt? Can you run pftt, apps compat and perf tests using this patch pls?

    Cheers,
    --
    Pierre

    @pierrejoye | http://www.libgd.org
  • Zeev Suraski at Oct 24, 2013 at 8:49 am

    -----Original Message-----
    From: Christopher Jones
    Sent: Thursday, October 24, 2013 3:17 AM
    To: Dmitry Stogov; Julien Pauli; David Soria Parra; Stas Malyshev; PHP
    Internals
    Subject: Re: [PHP-DEV] Improved performance of array_maerge() and
    func_get_args()


    On 10/23/2013 05:36 AM, Dmitry Stogov wrote:
    Hi,

    I'm proposing two simple patches that eliminate a lot of useless zval
    copying.
    For example they remove only about 800 calls to zend_hash_copy() (25%)
    on each request to wordpress-3.6.0 home page and make it 2-4% faster.

    It's not a questions about master branch, but I think it is also safe
    to commit them into PHP-5.4 and PHP-5.5.

    Any objections?

    https://gist.github.com/dstogov/7117623

    https://gist.github.com/dstogov/7117649

    Thanks. Dmitry.
    I'd prefer PHP 5.4 was kept stable.

    Last time I suggested keeping PHP 5.4 stable [1], it wasn't, and the
    result was
    a performance regression [2].
    I think the two are very different - this isn't some brand new complex
    algorithm, but a very localized optimization that provides net gains in some
    cases, with no real risk to other cases.

    As far as I understand our release rules, as long as we break no APIs and no
    ABIs - we can put it into presently shipping versions, as long as we're
    confident it won't introduce regressions (which is true for any bugfix).

    That said, I'm fine with us only putting it into 5.5 and not 5.4 to give
    people a bit more motivation to upgrade; It just doesn't make sense to sit
    on it for a whole year, when the risk associated with it isn't that higher
    than many other fixes we routinely introduce into bugfix releases all the
    time...

    Zeev
  • Julien Pauli at Oct 24, 2013 at 11:55 am

    On Thu, Oct 24, 2013 at 10:49 AM, Zeev Suraski wrote:

    -----Original Message-----
    From: Christopher Jones
    Sent: Thursday, October 24, 2013 3:17 AM
    To: Dmitry Stogov; Julien Pauli; David Soria Parra; Stas Malyshev; PHP
    Internals
    Subject: Re: [PHP-DEV] Improved performance of array_maerge() and
    func_get_args()


    On 10/23/2013 05:36 AM, Dmitry Stogov wrote:
    Hi,

    I'm proposing two simple patches that eliminate a lot of useless zval
    copying.
    For example they remove only about 800 calls to zend_hash_copy() (25%)
    on each request to wordpress-3.6.0 home page and make it 2-4% faster.

    It's not a questions about master branch, but I think it is also safe
    to commit them into PHP-5.4 and PHP-5.5.

    Any objections?

    https://gist.github.com/dstogov/7117623

    https://gist.github.com/dstogov/7117649

    Thanks. Dmitry.
    I'd prefer PHP 5.4 was kept stable.

    Last time I suggested keeping PHP 5.4 stable [1], it wasn't, and the
    result was
    a performance regression [2].
    I think the two are very different - this isn't some brand new complex
    algorithm, but a very localized optimization that provides net gains in
    some
    cases, with no real risk to other cases.

    As far as I understand our release rules, as long as we break no APIs and
    no
    ABIs - we can put it into presently shipping versions, as long as we're
    confident it won't introduce regressions (which is true for any bugfix).

    That said, I'm fine with us only putting it into 5.5 and not 5.4 to give
    people a bit more motivation to upgrade; It just doesn't make sense to sit
    on it for a whole year, when the risk associated with it isn't that higher
    than many other fixes we routinely introduce into bugfix releases all the
    time...
    I agree.
    I see no objection not to merge a patch that's been proven to give positive
    results.
    If test suite still pass, and numbers show a real improvement, including on
    real life projects, then just go merging it.

    I did not follow carefully the strstr patch, so I cannot give advice about
    it.

    Feel free to merge it to 5.5 and master.

    Julien Pauli
  • Christopher Jones at Oct 24, 2013 at 4:55 pm

    On 10/24/13 1:49 AM, Zeev Suraski wrote:
    -----Original Message-----
    From: Christopher Jones
    Sent: Thursday, October 24, 2013 3:17 AM
    To: Dmitry Stogov; Julien Pauli; David Soria Parra; Stas Malyshev; PHP
    Internals
    Subject: Re: [PHP-DEV] Improved performance of array_maerge() and
    func_get_args()


    On 10/23/2013 05:36 AM, Dmitry Stogov wrote:
    Hi,

    I'm proposing two simple patches that eliminate a lot of useless zval
    copying.
    For example they remove only about 800 calls to zend_hash_copy() (25%)
    on each request to wordpress-3.6.0 home page and make it 2-4% faster.

    It's not a questions about master branch, but I think it is also safe
    to commit them into PHP-5.4 and PHP-5.5.

    Any objections?

    https://gist.github.com/dstogov/7117623

    https://gist.github.com/dstogov/7117649

    Thanks. Dmitry.
    I'd prefer PHP 5.4 was kept stable.

    Last time I suggested keeping PHP 5.4 stable [1], it wasn't, and the
    result was
    a performance regression [2].
    I think the two are very different - this isn't some brand new complex
    algorithm, but a very localized optimization that provides net gains in some
    cases, with no real risk to other cases.

    As far as I understand our release rules, as long as we break no APIs and no
    ABIs - we can put it into presently shipping versions, as long as we're
    confident it won't introduce regressions (which is true for any bugfix).

    That said, I'm fine with us only putting it into 5.5 and not 5.4 to give
    people a bit more motivation to upgrade; It just doesn't make sense to sit
    on it for a whole year, when the risk associated with it isn't that higher
    than many other fixes we routinely introduce into bugfix releases all the
    time...

    Zeev
    I've no issue with it going to PHP 5.5.

    Chris

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupphp-internals @
categoriesphp
postedOct 23, '13 at 12:36p
activeOct 24, '13 at 4:55p
posts14
users8
websitephp.net

People

Translate

site design / logo © 2022 Grokbase