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

JoinIterator always calls hasNext on exhausted left iterator after ++ #9623

Closed
scabug opened this issue Jan 13, 2016 · 5 comments
Closed

JoinIterator always calls hasNext on exhausted left iterator after ++ #9623

scabug opened this issue Jan 13, 2016 · 5 comments
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented Jan 13, 2016

See demonstration of the issue.

The standard library method Iterator#++ creates a JoinIterator. The hasNext method on JoinIterator always calls the left iterator's hasNext method, rather than remembering that it has been exhausted.

In cases where the left iterator's hasNext method is expensive, such as any kind of IO, this was at the root of a baffling performance problem.

This behavior is fixed in ConcatIterator, which never calls the underlying hasNext a second time for an iterator which already returned false. So that provides an easy, if dirty, workaround:

val it1 = new IteratorWithExpensiveHasNext()
val it2 = new IteratorWithExpensiveHasNext()

// SLOW due to calling it1.hasNext every time
(it1 ++ it2).foreach(_ => ())

// FASTER due to NOT calling it1.hasNext every time
(Iterator.empty ++ it1 ++ it2).foreach(_ => ())

NOTE: you can use #memoizeExhaustion from my iterata project as a workaround, as well.

@scabug
Copy link
Author

scabug commented Jan 13, 2016

Imported From: https://issues.scala-lang.org/browse/SI-9623?orig=1
Reporter: @ms-tg
Affected Versions: 2.11.7
Other Milestones: 2.12.0-M4

@scabug
Copy link
Author

scabug commented Jan 29, 2016

@szeiger said:
Looks easily fixable but the ConcatIterator version is not much better. Iterating over 2 iterators of 3 elements each with JoinIterator calls hasNext (on either side) a total of 17 times, and it's still 14 times with ConcatIterator.

@scabug
Copy link
Author

scabug commented Jan 29, 2016

@ms-tg said:
Hi Stefan, can we ask for fixes that go into 2.11.x? Doesn't appear it should break binary compatibility?

@scabug
Copy link
Author

scabug commented Jan 29, 2016

@szeiger said:
PR: scala/scala#4931

@ms-tg, I thought it wouldn't be possible in 2.11 but I'll take another look.

@scabug scabug closed this as completed Feb 9, 2016
@scabug
Copy link
Author

scabug commented Mar 30, 2016

@adriaanm said:
See also scala/scala#5033 for a followup in 2.12.0-M4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants