Everything

php

Reusing main VM stack for generators

php.internals

Dmitry Stogov

10 years ago

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.

Bob Weinand

10 years ago

> 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

Dmitry Stogov

10 years ago

On 01/22/2016 10:29 PM, Bob Weinand wrote:

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

Bob Weinand

10 years ago

> 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

Dmitry Stogov

10 years ago

On 01/25/2016 11:36 AM, Bob Weinand wrote:

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