On Wed, 8 Nov 2023 17:24:45 GMT, Rémi Forax <fo...@openjdk.org> wrote:
>> This Pull-Request implements [JEP-461](https://openjdk.org/jeps/461) > > src/java.base/share/classes/java/util/stream/Gatherer.java line 530: > >> 528: * @param <R> the type of results this integrator can produce >> 529: */ >> 530: @ForceInline > > If we add this kind of the methods, we should add them on all function > interfaces of java.util.function and java.util.stream. Seems out of scope for this JEP > src/java.base/share/classes/java/util/stream/GathererOp.java line 50: > >> 48: */ >> 49: final class GathererOp<T, A, R> extends ReferencePipeline<T, R> { >> 50: @SuppressWarnings("unchecked") > > Could you explain why you need a SuppressWarnings here, especially the cast > to (ReferencePipeline<?,T>) in the else case Suggested improvements are most welcome! 👍 > src/java.base/share/classes/java/util/stream/GathererOp.java line 95: > >> 93: public void accept(X x) { >> 94: final var b = rightMost; >> 95: (b == null ? (rightMost = new NodeBuilder.Builder<>()) : >> b).accept(x); > > I think this code is unecessarily hard to read. > > var rightMost = this.rightMost; > if (rightMost == null) { > rightMost = this.rightMost = new NodeBuilder.Builder<>(); > } > rightMost.accept(x); Benchmark your change first. If it is cost-neutral I'm all for changing! > src/java.base/share/classes/java/util/stream/GathererOp.java line 180: > >> 178: finisher.accept(state, this); >> 179: sink.end(); >> 180: state = null; // GC assistance > > Not sure it really helps the GC, given that it may go through a GC barrier When it helps, it helps, and when it doesn't it has negligible cost. > src/java.base/share/classes/java/util/stream/GathererOp.java line 232: > >> 230: */ >> 231: @SuppressWarnings("unchecked") >> 232: private GathererOp(Gatherer<T, A, R> gatherer, GathererOp<?, ?, T> >> upstream) { > > Is it not better to use a static method named by example `fuse`, i don't > understand why it has to be a constructor. It is only ever exposed via a static method. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1386998422 PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1386999183 PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1387000558 PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1387002929 PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1387003542
