Skip to content

add stackfull coroutine(Fiber) support #2723

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from
Closed

add stackfull coroutine(Fiber) support #2723

wants to merge 6 commits into from

Conversation

taoso
Copy link

@taoso taoso commented Aug 31, 2017

Make this PR for comment.

@KalleZ
Copy link
Member

KalleZ commented Aug 31, 2017

I'm a little concerned on this at that it adds a new keyword but it doesn't have any tests at all

@taoso
Copy link
Author

taoso commented Aug 31, 2017

Hello @KalleZ , thanks for commenting. This is PR is now a prototype and for comment only. I had send a discuss email to the internal list. If this feature will be accepted, I will add tests.

@KalleZ
Copy link
Member

KalleZ commented Aug 31, 2017

@lvht thanks! I will have a look once more updates come :)

@KalleZ
Copy link
Member

KalleZ commented Aug 31, 2017

Ps. remember to update ext/tokenizer (and tests) whenever

@taoso
Copy link
Author

taoso commented Aug 31, 2017

hello @KalleZ , I have made some basic test to demonstrate the fiber api. And the tokenizer has been updated as well.

@nikic
Copy link
Member

nikic commented Sep 1, 2017

What happens if there are internal calls on the call stack? Say something like array_map(function() { await; }, [1, 2, 3]); inside a fiber. Internal calls (using the C stack rather than the VM stack) are usually the problem with this kind of endeavor.

@laruence
Copy link
Member

laruence commented Sep 1, 2017

First this PR should against master branch as it include so many changes, especially lots changes in zend VM.

Second, all changes to VM seems only relates to AWAIT, that make me think, maybe this feature could exists as an class lay in ext/spl, or even an PECL extension? you only need change await to Fiber::await(), right? and in that case, this feature will be self-contain, and more robust to me.

thanks

@taoso
Copy link
Author

taoso commented Sep 1, 2017

@nikic It is a big shot to me. And it seems that we cannot restore/resume the stack in zend engine.
Is it proper to throw an exception when await is used in the internal call?

@taoso
Copy link
Author

taoso commented Sep 1, 2017

Hello @laruence , I want to make the Fiber as a pecl extension. But is there any approach to pause the zend engine without introducing a new opcode?

@laruence
Copy link
Member

laruence commented Sep 1, 2017

@lvht in theory, I think yes, you could use longjmp, save context(zend_execute_data), then restore it when doing resume... anyway, I haven't verify it ..

@taoso taoso mentioned this pull request Sep 2, 2017
@taoso taoso closed this Sep 2, 2017
@taoso
Copy link
Author

taoso commented Sep 2, 2017

please see #2733

@taoso taoso mentioned this pull request Apr 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants