Hi,
Could you please, take a look into the PoC.
It's incomplete, but may be you get ideas how to fix this.
https://gist.github.com/dstogov/285024375d15cacf2a9b
Few tests are failed, because YIELD may be used as expression in nested calls (between INIT_FCALL/DO_FCALL), and the caller function restores EG(vm_stack_top), loosing and overwriting that active frame.
<?php
function gen() {
var_dump(yield 1);
}
for ($gen = gen(); $gen->valid(); $gen->send(0)) {
var_dump($gen->current());
}
?>
Thanks. Dmitry.
Am 22.01.2016 um 15:43 schrieb Dmitry Stogov dmitry@zend.com:
Hi,
Could you please, take a look into the PoC.
It's incomplete, but may be you get ideas how to fix this.https://gist.github.com/dstogov/285024375d15cacf2a9b https://gist.github.com/dstogov/285024375d15cacf2a9b
Few tests are failed, because YIELD may be used as expression in nested calls (between INIT_FCALL/DO_FCALL), and the caller function restores EG(vm_stack_top), loosing and overwriting that active frame.
<?php
function gen() {
var_dump(yield 1);
}for ($gen = gen(); $gen->valid(); $gen->send(0)) {
var_dump($gen->current());
}
?>Thanks. Dmitry.
Hey, that's pretty similar to what I did https://github.com/php/php-src/commit/0f0471a9893230918084798d4e35db37d7b4519d https://github.com/php/php-src/compare/master...bwoebi:stackless_generators in that old patch, which you rejected because we hadn't found any solution for that issue with yield inside calls.
We basically need a way to properly first fetch the args (beware: func_arg fetches...) before instantiating the call frame, an issue which for example could be solved if we applied my vm_stack_restructuring patch (because it only installs call frames after all args were fetched).
I believe this patch won't work without significant refactoring (like vm_stack_refactoring branch) … so, either we bite the bullet and look at that patch again, or we can forget this idea too.
Thanks,
Bob
Am 22.01.2016 um 15:43 schrieb Dmitry Stogov <dmitry@zend.com
mailto:dmitry@zend.com>:Hi,
Could you please, take a look into the PoC.
It's incomplete, but may be you get ideas how to fix this.https://gist.github.com/dstogov/285024375d15cacf2a9b
Few tests are failed, because YIELD may be used as expression in
nested calls (between INIT_FCALL/DO_FCALL), and the caller function
restores EG(vm_stack_top), loosing and overwriting that active frame.<?php
function gen() {
var_dump(yield 1);
}for ($gen = gen(); $gen->valid(); $gen->send(0)) {
var_dump($gen->current());
}
?>Thanks. Dmitry.
Hey, that's pretty similar to what I did
https://github.com/php/php-src/commit/0f0471a9893230918084798d4e35db37d7b4519d
https://github.com/php/php-src/compare/master...bwoebi:stackless_generators in
that old patch, which you rejected because we hadn't found any
solution for that issue with yield inside calls.
yes, this is similar.
We basically need a way to properly first fetch the args (beware:
func_arg fetches...) before instantiating the call frame, an issue
which for example could be solved if we applied my
vm_stack_restructuring patch (because it only installs call frames
after all args were fetched).
I thought about that approach, but the amount of changes doesn't worth
the result. Doing this only for generators doesn't make a lot of sense.
I believe this patch won't work without significant refactoring (like
vm_stack_refactoring branch) … so, either we bite the bullet and look
at that patch again, or we can forget this idea too.
most probably, you are right.
Thanks. Dmitry.
Thanks,
Bob
Am 25.01.2016 um 09:13 schrieb Dmitry Stogov dmitry@zend.com:
We basically need a way to properly first fetch the args (beware: func_arg fetches...) before instantiating the call frame, an issue which for example could be solved if we applied my vm_stack_restructuring patch (because it only installs call frames after all args were fetched).
I thought about that approach, but the amount of changes doesn't worth the result. Doing this only for generators doesn't make a lot of sense.
After all, we still have that 2% performance improvement. And the main VM stack reusage for Generators.
And I believe the proposed layout is also much more close to the C calling convention and thus more compatible if we ever go the route down to compiling to assembly.
Ultimately it gives us more flexibility as we also could put point the return temporary/cv of some opcodes to the right place, ready to call.
We aren't that far yet with the Optimizer, but I think I see it coming, where we'll benefit more from fixed places. It might be just a few moves less, but function arg passing is very common in PHP, so the benefit will be more significant the more optimizations we apply.
Remember that small thing removing ZEND_SEND_VAL_SPEC_TMP_TMP_HANDLERs where unnecessary? When we'll be able to know which variables may hold references or which not, we'll suddenly be able to directly write to the function parameter slot in much more cases.
I still stand by the point that doing it now (which small, but non-negligible improvement), will bring us a bigger improvement later. Many small improvements built around this patch will give us a big one in the end.
Thanks,
Bob
Am 25.01.2016 um 09:13 schrieb Dmitry Stogov dmitry@zend.com:
We basically need a way to properly first fetch the args (beware: func_arg fetches...) before instantiating the call frame, an issue which for example could be solved if we applied my vm_stack_restructuring patch (because it only installs call frames after all args were fetched).
I thought about that approach, but the amount of changes doesn't worth the result. Doing this only for generators doesn't make a lot of sense.
After all, we still have that 2% performance improvement. And the main VM stack reusage for Generators.And I believe the proposed layout is also much more close to the C calling convention and thus more compatible if we ever go the route down to compiling to assembly.
Ultimately it gives us more flexibility as we also could put point the return temporary/cv of some opcodes to the right place, ready to call.
We aren't that far yet with the Optimizer, but I think I see it coming, where we'll benefit more from fixed places. It might be just a few moves less, but function arg passing is very common in PHP, so the benefit will be more significant the more optimizations we apply.Remember that small thing removing ZEND_SEND_VAL_SPEC_TMP_TMP_HANDLERs where unnecessary? When we'll be able to know which variables may hold references or which not, we'll suddenly be able to directly write to the function parameter slot in much more cases.
I still stand by the point that doing it now (which small, but non-negligible improvement), will bring us a bigger improvement later. Many small improvements built around this patch will give us a big one in the end.
I put a note about "postponed" idea at https://wiki.php.net/php-7.1-ideas.
May be we will return to this later.
Thanks. Dmitry.
Thanks. Dmitry.
Thanks,
Bob