Make Stream.withFilter.{map,flatMap} run in constant stack space #1167
Conversation
The included test currently fails because `map` and `flatMap` do not run in constant stack space on a stream returned by `Stream.withFilter`, as I reported here: https://groups.google.com/d/msg/scala-language/WqJR38REXnk/saaSiDdmyqoJ Fix the problem and add a simple testcase. Note that the stack space consumed when producing an element of this stream is proportional to the number of elements failing the test before the next success. The stack space consumed to produce the stream itself is the space needed to produce the first element, that is, is proportional to the number of failures before the first success.
| assert(resMap1.isEmpty) | ||
| assert(resFMap1.isEmpty) | ||
| println(resMap1) | ||
| println(resFMap1) |
There was a problem hiding this comment.
On second thought, I could remove these lines and have no output at all. I'll push a new commit if this is preferred.
|
Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/249/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/956/ |
| assert(resFMap1.isEmpty) | ||
| println(resMap1) | ||
| println(resFMap1) | ||
| //This will cause a stack overflow |
There was a problem hiding this comment.
Here I meant "with the previous implementation".
No need to check the output - checking programmatically that the produced streams are empty is enough.
| ) | ||
| */ |
There was a problem hiding this comment.
Commenting out code instead of removing it was not my intention. Will fix.
|
jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/956/ |
|
jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/249/ |
|
PLS REBUILD ALL |
|
Note: the test results are, strictly speaking, not valid any more, even though the changes are minor. I would have deleted the comments, as required by the pull request policy, but I don't have permissions for that. |
|
Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/255/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/962/ |
|
jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/255/ |
|
jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/962/ |
|
I believe this would benefit from additional correctness test cases such as calling
In all cases, the predicate for After that, LGTM. |
Test a wider range of functionality.
|
I wrote the test case you asked for - however, the "expected" result is computed by converting the stream to a Seq (and checking that the result is right) and using filter, map and flatMap on Seq. I thus assume that those functions on Seq are are tested independently. If that's a problem I could list instead the results explicitly, even though that would make additional testcases harder to add. Or I could additionally print the results (and have a corresponding .check file). |
|
PLS REBUILD ALL |
|
Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/266/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/973/ |
|
LGTM |
|
jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/973/ |
|
jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/266/ |
Make Stream.withFilter.{map,flatMap} run in constant stack space
The included test currently fails because
mapandflatMapdo notrun in constant stack space on a stream returned by
Stream.withFilter,as I reported here:
https://groups.google.com/d/msg/scala-language/WqJR38REXnk/saaSiDdmyqoJ
Fix the problem and add a simple testcase.
Note that the stack space consumed when producing an element of this stream is
proportional to the number of elements failing the test before the next
success. The stack space consumed to produce the stream itself is the
space needed to produce the first element, that is, is proportional to
the number of failures before the first success.
Review by @axel22.