Skip to content

Make f(await(completedFuture)) execute f synchronously #98

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

Merged
merged 3 commits into from
Dec 18, 2014

Conversation

retronym
Copy link
Member

A worthy optimization, suggested by @danarmak.

Closes #73

Previously, as sequence of state transitions that did not pass through
an asynchrous boundary incurred stack frames. The trivial loop in
the enclosed test case would then overflow the stack.

This commit merges the `resume` and `apply(tr: Try[Any])` methods into
a `apply`. It changes the body of this method to be an infinite loop
with returns at the terminal points in the state machine (or at a
terminal failure.)

To allow merging of these previously separate matches, states that
contain an await are now allocated two state ids: one for the setup
code that calls `onComplete`, and one for the code in the continuation
that records the result and advances the state machine.

Fixes scala#93
@retronym
Copy link
Member Author

Review by @danarmak @phaller

@retronym
Copy link
Member Author

/cc @viktorklang

future.splice.isCompleted
}
override def getCompleted[A: WeakTypeTag](future: Expr[Fut[A]]): Expr[Tryy[A]] = reify {
future.splice.value.get
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a heads up: One has to assume that .value.get will allocate that Option.

@phaller
Copy link
Contributor

phaller commented Dec 17, 2014

LGTM

retronym added a commit that referenced this pull request Dec 18, 2014
Make `f(await(completedFuture))` execute `f` synchronously
@retronym retronym merged commit 41ae707 into scala:2.10.x Dec 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants