Skip to content
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

New ponyrt #363

Merged
merged 69 commits into from
Apr 7, 2016
Merged

New ponyrt #363

merged 69 commits into from
Apr 7, 2016

Conversation

albertnetymk
Copy link
Contributor

Long awaited new ponyrt update.

@kikofernandez
Copy link
Contributor

To make this completely clear, we had to disable async's and foreach, as the tasks require some re-thinking and much more testing.

@kikofernandez
Copy link
Contributor

This PR has been rebased and I am merging in 10' unless someone opposes to this PR

@EliasC
Copy link
Contributor

EliasC commented Apr 7, 2016

I'm all for merging it! Would it possible to get a short summary of the (important) changes made to make it easier for future hackers who were not involved in this migration?

@albertnetymk
Copy link
Contributor Author

My attempt:

  • add ctx extra argument for all almost all functions to avoid calling pony_ctx()
  • realize that ctx passed as argument can't be used, for it could be obsolete due to stack manipulation in encore thread
  • use oo structure for generated code so that ctx returned from pony_ctx() could be re-used to some extent
  • small change of actor processing hooks due to msg batching in ponyrt
  • aforementioned tasks related stuff

@kikofernandez kikofernandez merged commit 57e1173 into development Apr 7, 2016
@albertnetymk albertnetymk deleted the new-ponyrt branch April 7, 2016 12:09
@EliasC
Copy link
Contributor

EliasC commented Apr 7, 2016

@albertnetymk: Does the code generation need to be aware of when the ctx argument can be used or not, or is it encapsulated in the runtime?

@albertnetymk
Copy link
Contributor Author

CG always emits the extra ctx arg, and the runtime decides to use it or not accordingly.

@EliasC
Copy link
Contributor

EliasC commented Apr 7, 2016

Sounds good! Thanks!

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