> Sometimes, `Spliterator::forEachRemaining` has much more optimized 
> implementation, compared to `Spliterator::tryAdvance`. For example, if a 
> `Stream::spliterator` called for a stream that has intermediate operations, 
> `tryAdvance()` may buffer intermediate elements while `forEachRemaining()` 
> just delegates to the `wrapAndCopyInto` (see 
> `StreamSpliterators.AbstractWrappingSpliterator` and its inheritors).
> 
> `Spliterators::iterator` methods (used in particular by `Stream::iterator`) 
> provide adapter iterators that delegate to the supplied spliterator. 
> Unfortunately, they don't have a specialized implementation for 
> `Iterator::forEachRemaining`. As a result, a default implementation is used 
> that delegates to `hasNext`/`next`, which in turn causes the delegation to 
> `tryAdvance`. It's quite simple to implement `Iterator::forEachRemaining` 
> here, taking advantage of possible spliterator optimizations if the iterator 
> client decides to use `forEachRemaining`.
> 
> This patch implements Iterator::forEachRemaining in Spliterators::iterator 
> methods. Also, I nullize the `nextElement` in `Iterator::next` to avoid 
> accidental prolongation of the user's object lifetime when iteration is 
> finished. Finally, a small cleanup: I added the `final` modifier where 
> possible to private fields in `Spliterators`.
> 
> Test-wise, some scenarios are already covered by 
> SpliteratorTraversingAndSplittingTest. However, the resulting iterator is 
> always wrapped into `Spliterators::spliterator`, so usage scenarios are 
> somewhat limited. In particular, calling `hasNext` (without `next`) before 
> `forEachRemaining` was not covered there. I added more tests in 
> `IteratorFromSpliteratorTest` to cover these scenarios. I checked that 
> removing `valueReady = false;` or `action.accept(t);` lines from newly 
> implemented `forEachRemaining` method causes new tests to fail (but old tests 
> don't fail due to this).

Tagir F. Valeev has updated the pull request incrementally with one additional 
commit since the last revision:

  Add a comment clarifying the purpose of the test

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4124/files
  - new: https://git.openjdk.java.net/jdk/pull/4124/files/9a5e0dd2..cc337995

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4124&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4124&range=01-02

  Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4124.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4124/head:pull/4124

PR: https://git.openjdk.java.net/jdk/pull/4124

Reply via email to