Hi,
I propose a micro optimization for RETURN statement.
Currently "return $x" increments reference counter of $x, then in zend_leave_helper() we perform zval_ptr_dtor() on the same $x.
The patch sets the original value of $x to null in first place, so zval_ptr_dtor() is not going to be called.
https://gist.github.com/dstogov/36f68b206242a39691ac539c2fc85d40
the performance impact is invisible (0.1% less instruction retired on Wordpress).
It breaks sapi/phpdbg/tests/breakpoints_005.phpt, but this is probably not a big deal.
BTW: this change may affect debuggers in some other way.
Let me know, if you see any problems (I'll delay commit for 2-3 days).
Thanks. Dmitry.
Hi,
I propose a micro optimization for RETURN statement.
Currently "return $x" increments reference counter of $x, then in zend_leave_helper() we perform zval_ptr_dtor() on the same $x.
The patch sets the original value of $x to null in first place, so zval_ptr_dtor() is not going to be called.
https://gist.github.com/dstogov/36f68b206242a39691ac539c2fc85d40
the performance impact is invisible (0.1% less instruction retired on Wordpress).
It breaks sapi/phpdbg/tests/breakpoints_005.phpt, but this is probably not a big deal.
BTW: this change may affect debuggers in some other way.
I suppose it is for 7.1 anyway, right? so debugger will have to be
ported but it would be nice to know exactly how this change will
affect them to make sure that users can migrate to 7.1 smoothly and
still use their tools.
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
I propose a micro optimization for RETURN statement.
Currently "return $x" increments reference counter of $x, then in
zend_leave_helper() we perform zval_ptr_dtor() on the same $x.The patch sets the original value of $x to null in first place, so
zval_ptr_dtor() is not going to be called.https://gist.github.com/dstogov/36f68b206242a39691ac539c2fc85d40
the performance impact is invisible (0.1% less instruction retired on
Wordpress).It breaks sapi/phpdbg/tests/breakpoints_005.phpt, but this is probably
not a big deal.BTW: this change may affect debuggers in some other way.
I'd like to know why this breaks before saying something. It'd be a PITA
if this micro optimisation wouldn't actually do a lot performance wise,
but makes some debugging not possible.
cheers,
Derick
Morning Derick,
I don't think it does make anything impossible, it's just a more
efficient copying method in the EXPECTED branch is all.
Cheers
Joe
I propose a micro optimization for RETURN statement.
Currently "return $x" increments reference counter of $x, then in
zend_leave_helper() we perform zval_ptr_dtor() on the same $x.The patch sets the original value of $x to null in first place, so
zval_ptr_dtor() is not going to be called.https://gist.github.com/dstogov/36f68b206242a39691ac539c2fc85d40
the performance impact is invisible (0.1% less instruction retired on
Wordpress).It breaks sapi/phpdbg/tests/breakpoints_005.phpt, but this is probably
not a big deal.BTW: this change may affect debuggers in some other way.
I'd like to know why this breaks before saying something. It'd be a PITA
if this micro optimisation wouldn't actually do a lot performance wise,
but makes some debugging not possible.cheers,
Derick
But somehow it broke one phpdbg test, so it's better to check.
Thanks. Dmitry.
Morning Derick,
I don't think it does make anything impossible, it's just a more
efficient copying method in the EXPECTED branch is all.
Cheers
JoeOn Tue, Apr 5, 2016 at 10:04 AM, Derick Rethans <derick@php.net
mailto:derick@php.net> wrote:> I propose a micro optimization for RETURN statement. > > Currently "return $x" increments reference counter of $x, then in > zend_leave_helper() we perform zval_ptr_dtor() on the same $x. > > The patch sets the original value of $x to null in first place, so > zval_ptr_dtor() is not going to be called. > > https://gist.github.com/dstogov/36f68b206242a39691ac539c2fc85d40 > > the performance impact is invisible (0.1% less instruction retired on > Wordpress). > > It breaks sapi/phpdbg/tests/breakpoints_005.phpt, but this is probably > not a big deal. > > BTW: this change may affect debuggers in some other way. I'd like to know why this breaks before saying something. It'd be a PITA if this micro optimisation wouldn't actually do a lot performance wise, but makes some debugging not possible. cheers, Derick
I propose a micro optimization for RETURN statement.
Currently "return $x" increments reference counter of $x, then in
zend_leave_helper() we perform zval_ptr_dtor() on the same $x.The patch sets the original value of $x to null in first place, so
zval_ptr_dtor() is not going to be called.https://gist.github.com/dstogov/36f68b206242a39691ac539c2fc85d40
the performance impact is invisible (0.1% less instruction retired on
Wordpress).It breaks sapi/phpdbg/tests/breakpoints_005.phpt, but this is probably
not a big deal.BTW: this change may affect debuggers in some other way.
I'd like to know why this breaks before saying something. It'd be a PITA
if this micro optimisation wouldn't actually do a lot performance wise,
but makes some debugging not possible.
Actually, the patch has a bug.
It doesn't take into account that "return $x;" might be used in global
scope.
Setting $x toNULL
in this case isn't right, of course.
Introducing another check would probably negate the effect of
micro-optimization.
So, ignore this for now, and sorry for noise.
Thanks. Dmitry.
cheers,
Derick
Dmitry,
No worries ... I had just found out it wasn't particular to phpdbg ...
<?php
function foo($bar) {
var_dump(eval ("return $bar;"));
var_dump($bar);
}
foo("test");
?>
Cheers
Joe
I propose a micro optimization for RETURN statement.
Currently "return $x" increments reference counter of $x, then in
zend_leave_helper() we perform zval_ptr_dtor() on the same $x.The patch sets the original value of $x to null in first place, so
zval_ptr_dtor() is not going to be called.https://gist.github.com/dstogov/36f68b206242a39691ac539c2fc85d40
the performance impact is invisible (0.1% less instruction retired on
Wordpress).It breaks sapi/phpdbg/tests/breakpoints_005.phpt, but this is probably
not a big deal.BTW: this change may affect debuggers in some other way.
I'd like to know why this breaks before saying something. It'd be a PITA
if this micro optimisation wouldn't actually do a lot performance wise,
but makes some debugging not possible.Actually, the patch has a bug.
It doesn't take into account that "return $x;" might be used in global
scope.
Setting $x toNULL
in this case isn't right, of course.Introducing another check would probably negate the effect of
micro-optimization.So, ignore this for now, and sorry for noise.
Thanks. Dmitry.
cheers,
Derick