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.

Sign up for GitHub

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

Jump to bottom

Fix lazy evaluation of StreamWithFilter#foreach #8990

Closed
scabug opened this issue Nov 17, 2014 · 7 comments
Closed

Fix lazy evaluation of StreamWithFilter#foreach #8990

scabug opened this issue Nov 17, 2014 · 7 comments
Milestone
2.12.0-M1

Comments

@scabug
Copy link

scabug commented Nov 17, 2014

This bug seems to be caused by the alternate, non-recursive implementation of #foreach in StreamWithFilter, which ends up holding a reference to the beginning of the stream for the lifetime of the iteration.

Welcome to Scala version 2.10.4 (Java HotSpot(TM) 64-Bit Server VM, Java 1.7.0_25).
Type in expressions to have them evaluated.
Type :help for more information.

scala> Stream.continually(42).take(100000000).foreach(_ => ()) // NO PROBLEM

scala> Stream.continually(42).take(100000000).withFilter(_ => true).foreach(_ => ())
java.lang.OutOfMemoryError: Java heap space
	at scala.collection.immutable.Stream.take(Stream.scala:731)
	at scala.collection.immutable.Stream$$anonfun$take$2.apply(Stream.scala:731)
	at scala.collection.immutable.Stream$$anonfun$take$2.apply(Stream.scala:731)
	at scala.collection.immutable.Stream$Cons.tail(Stream.scala:1085)
	at scala.collection.immutable.Stream$Cons.tail(Stream.scala:1077)
	at scala.collection.immutable.Stream$StreamWithFilter.foreach(Stream.scala:523)
	at .<init>(<console>:8)
	at .<clinit>(<console>)
	at .<init>(<console>:7)
	at .<clinit>(<console>)
	at $print(<console>)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:606)
	at scala.tools.nsc.interpreter.IMain$ReadEvalPrint.call(IMain.scala:734)
	at scala.tools.nsc.interpreter.IMain$Request.loadAndRun(IMain.scala:983)
	at scala.tools.nsc.interpreter.IMain.loadAndRunReq$1(IMain.scala:573)
	at scala.tools.nsc.interpreter.IMain.interpret(IMain.scala:604)
	at scala.tools.nsc.interpreter.IMain.interpret(IMain.scala:568)
	at scala.tools.nsc.interpreter.ILoop.reallyInterpret$1(ILoop.scala:760)
	at scala.tools.nsc.interpreter.ILoop.interpretStartingWith(ILoop.scala:805)
	at scala.tools.nsc.interpreter.ILoop.command(ILoop.scala:717)
	at scala.tools.nsc.interpreter.ILoop.processLine$1(ILoop.scala:581)
	at scala.tools.nsc.interpreter.ILoop.innerLoop$1(ILoop.scala:588)
	at scala.tools.nsc.interpreter.ILoop.loop(ILoop.scala:591)
	at scala.tools.nsc.interpreter.ILoop$$anonfun$process$1.apply$mcZ$sp(ILoop.scala:882)
	at scala.tools.nsc.interpreter.ILoop$$anonfun$process$1.apply(ILoop.scala:837)
	at scala.tools.nsc.interpreter.ILoop$$anonfun$process$1.apply(ILoop.scala:837)
	at scala.tools.nsc.util.ScalaClassLoader$.savingContextLoader(ScalaClassLoader.scala:135)
	at scala.tools.nsc.interpreter.ILoop.process(ILoop.scala:837)
	at scala.tools.nsc.interpreter.ILoop.main(ILoop.scala:904)
@scabug
Copy link
Author

scabug commented Nov 17, 2014

Imported From: https://issues.scala-lang.org/browse/SI-8990?orig=1
Reporter: @ms-tg
Assignee: @ms-tg
Affected Versions: 2.10.4, 2.11.4

@scabug
Copy link
Author

scabug commented Jan 30, 2015

@Ichoran said:
@ms-tg - Do you want to fix this or shall I?

@scabug
Copy link
Author

scabug commented Jan 31, 2015

@ms-tg said (edited on Jan 31, 2015 3:12:09 AM UTC):
Hi Rex Kerr, if it's ok with you, I'd love a chance to make the fix myself -- I am finally setup and can run tests, and am on the way :)

@scabug
Copy link
Author

scabug commented Jan 31, 2015

@Ichoran said:
@ms-tg Sure, go for it!

@scabug
Copy link
Author

scabug commented Jan 31, 2015

@ms-tg said:
I've posted a beginning PR here: scala/scala#4277

As explained there and on scala-internals, I'm a bit stymied by the baked-in
reference to the head of the Stream in StreamWithFilter. As far as I can see,
this is due to the heritage from TraversableLike#WithFilter, which is also an
inner class which closes over the instance of the outer class.

Do you have any suggestions on approaches I could take here? The two tests
I added in that pull request are in test/junit/scala/collection/immutable/StreamTest.scala.

Thanks for any ideas,
-Marc

@scabug
Copy link
Author

scabug commented Feb 4, 2015

@adriaanm said:
See also #9134

@scabug
Copy link
Author

scabug commented Feb 23, 2015

@adriaanm said:
scala/scala#4284

@scabug scabug closed this as completed Feb 23, 2015
@scabug scabug added this to the 2.12.0-M1 milestone Apr 7, 2017
This was referenced Apr 7, 2017
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.12.0-M1
Development

No branches or pull requests
1 participant
@scabug