-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Enhances stack safety for Eval. #1888
Merged
Merged
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
045b195
Add a stack safety stress test for Eval.
non cebeb4e
Merge remote-tracking branch 'upstream/master' into topic/eval-stack-…
erik-stripe 52ca2ca
Reduce depth to get Travis passing.
erik-stripe 12d6fd0
Better memoization support for flatMap.
erik-stripe c9572b2
Rename Compute to FlatMap and Call to Defer.
erik-stripe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -72,28 +72,28 @@ sealed abstract class Eval[+A] extends Serializable { self => | |
*/ | ||
def flatMap[B](f: A => Eval[B]): Eval[B] = | ||
this match { | ||
case c: Eval.Compute[A] => | ||
new Eval.Compute[B] { | ||
case c: Eval.FlatMap[A] => | ||
new Eval.FlatMap[B] { | ||
type Start = c.Start | ||
// See https://issues.scala-lang.org/browse/SI-9931 for an explanation | ||
// of why the type annotations are necessary in these two lines on | ||
// Scala 2.12.0. | ||
val start: () => Eval[Start] = c.start | ||
val run: Start => Eval[B] = (s: c.Start) => | ||
new Eval.Compute[B] { | ||
new Eval.FlatMap[B] { | ||
type Start = A | ||
val start = () => c.run(s) | ||
val run = f | ||
} | ||
} | ||
case c: Eval.Call[A] => | ||
new Eval.Compute[B] { | ||
case c: Eval.Defer[A] => | ||
new Eval.FlatMap[B] { | ||
type Start = A | ||
val start = c.thunk | ||
val run = f | ||
} | ||
case _ => | ||
new Eval.Compute[B] { | ||
new Eval.FlatMap[B] { | ||
type Start = A | ||
val start = () => self | ||
val run = f | ||
|
@@ -203,7 +203,7 @@ object Eval extends EvalInstances { | |
* which produces an Eval[A] value. Like .flatMap, it is stack-safe. | ||
*/ | ||
def defer[A](a: => Eval[A]): Eval[A] = | ||
new Eval.Call[A](a _) {} | ||
new Eval.Defer[A](a _) {} | ||
|
||
/** | ||
* Static Eval instance for common value `Unit`. | ||
|
@@ -246,56 +246,60 @@ object Eval extends EvalInstances { | |
val One: Eval[Int] = Now(1) | ||
|
||
/** | ||
* Call is a type of Eval[A] that is used to defer computations | ||
* Defer is a type of Eval[A] that is used to defer computations | ||
* which produce Eval[A]. | ||
* | ||
* Users should not instantiate Call instances themselves. Instead, | ||
* Users should not instantiate Defer instances themselves. Instead, | ||
* they will be automatically created when needed. | ||
*/ | ||
sealed abstract class Call[A](val thunk: () => Eval[A]) extends Eval[A] { | ||
sealed abstract class Defer[A](val thunk: () => Eval[A]) extends Eval[A] { | ||
|
||
def memoize: Eval[A] = Memoize(this) | ||
def value: A = evaluate(this) | ||
} | ||
|
||
/** | ||
* Collapse the call stack for eager evaluations. | ||
* returns a non Call Eval node | ||
* Advance until we find a non-deferred Eval node. | ||
* | ||
* Often we may have deep chains of Defer nodes; the goal here is to | ||
* advance through those to find the underlying "work" (in the case | ||
* of FlatMap nodes) or "value" (in the case of Now, Later, or | ||
* Always nodes). | ||
*/ | ||
@tailrec private def doCall[A](fa: Eval[A]): Eval[A] = | ||
@tailrec private def advance[A](fa: Eval[A]): Eval[A] = | ||
fa match { | ||
case call: Eval.Call[A] => | ||
doCall(call.thunk()) | ||
case compute: Eval.Compute[A] => | ||
new Eval.Compute[A] { | ||
case call: Eval.Defer[A] => | ||
advance(call.thunk()) | ||
case compute: Eval.FlatMap[A] => | ||
new Eval.FlatMap[A] { | ||
type Start = compute.Start | ||
val start: () => Eval[Start] = () => compute.start() | ||
val run: Start => Eval[A] = s => doCall1(compute.run(s)) | ||
val run: Start => Eval[A] = s => advance1(compute.run(s)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need this here? Seems like it will be run later without beating the function call here. |
||
} | ||
case other => other | ||
} | ||
|
||
/** | ||
* Alias for doCall that can be called in a non-tail position | ||
* from an otherwise tailrec-optimized doCall. | ||
* Alias for advance that can be called in a non-tail position | ||
* from an otherwise tailrec-optimized advance. | ||
*/ | ||
private def doCall1[A](fa: Eval[A]): Eval[A] = | ||
doCall(fa) | ||
private def advance1[A](fa: Eval[A]): Eval[A] = | ||
advance(fa) | ||
|
||
/** | ||
* Compute is a type of Eval[A] that is used to chain computations | ||
* FlatMap is a type of Eval[A] that is used to chain computations | ||
* involving .map and .flatMap. Along with Eval#flatMap it | ||
* implements the trampoline that guarantees stack-safety. | ||
* | ||
* Users should not instantiate Compute instances | ||
* Users should not instantiate FlatMap instances | ||
* themselves. Instead, they will be automatically created when | ||
* needed. | ||
* | ||
* Unlike a traditional trampoline, the internal workings of the | ||
* trampoline are not exposed. This allows a slightly more efficient | ||
* implementation of the .value method. | ||
*/ | ||
sealed abstract class Compute[A] extends Eval[A] { self => | ||
sealed abstract class FlatMap[A] extends Eval[A] { self => | ||
type Start | ||
val start: () => Eval[Start] | ||
val run: Start => Eval[A] | ||
|
@@ -330,9 +334,9 @@ object Eval extends EvalInstances { | |
|
||
@tailrec def loop(curr: L, fs: List[C]): Any = | ||
curr match { | ||
case c: Compute[_] => | ||
case c: FlatMap[_] => | ||
c.start() match { | ||
case cc: Compute[_] => | ||
case cc: FlatMap[_] => | ||
loop( | ||
cc.start().asInstanceOf[L], | ||
cc.run.asInstanceOf[C] :: c.run.asInstanceOf[C] :: fs) | ||
|
@@ -346,8 +350,8 @@ object Eval extends EvalInstances { | |
case xx => | ||
loop(c.run(xx.value), fs) | ||
} | ||
case call: Call[_] => | ||
loop(doCall(call), fs) | ||
case call: Defer[_] => | ||
loop(advance(call), fs) | ||
case m@Memoize(eval) => | ||
m.result match { | ||
case Some(a) => | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
totally nitpick:
call
can be renamed todefer
and thecompute
below. No need to address in this PR, I can do in a separate one.