Hi Viktor. I guess for 1-4 I'm just looking for a quick eyeball to check if my takeaways are valid, nothing too committal.
I'm a little more interested in 5, because I'm currently building a document that is attempting to capture how the stream internals fit together, including visualizing stream performance characteristics. How op flags are set/cleared/propagated plays a big part in what I've prepared. I want to make sure I don't oversimplify how they work. I have not attempted to make/test changes to the source. I guess I didn't presume I could contribute in that way, but I can look into it if you think it's worth it :) Thanks, Daniel On Fri, Jan 17, 2025 at 3:32 AM Viktor Klang <viktor.kl...@oracle.com> wrote: > Hi Daniel, > > Thanks for your patience. > > What kind of feedback/responses are you looking for primarily? > > Have you attempted to make changes and run any of the openjdk stream > benches (after running the tests)? > > Cheers, > √ > > > *Viktor Klang* > Software Architect, Java Platform Group > Oracle > ------------------------------ > *From:* core-libs-dev <core-libs-dev-r...@openjdk.org> on behalf of > Daniel Avery <danielave...@gmail.com> > *Sent:* Friday, 10 January 2025 04:01 > *To:* core-libs-dev@openjdk.org <core-libs-dev@openjdk.org> > *Subject:* Minor optimizations / questions about stream implementation > > Hi, > > I recently finished a deep-dive through the code in java.util.stream, and > came across a few corners that stuck out to me as being odd, in a "small > missed opportunity" kind of way. I figured I'd raise them here just in case > any are legitimate, or I could get some better insight. I am looking at JDK > 24 [build 30] ( > https://github.com/openjdk/jdk/tree/jdk-24%2B30/src/java.base/share/classes/java > ). > > 1. AbstractPipeline.wrap() is passed isParallel(), which ultimately (only) > plays into determining if StreamSpliterators.AbstractWrappingSpliterator is > allowed to split. It looks like the code could be more lenient, and instead > pass (isParallel() || !hasAnyStateful()), i.e. allow splitting the > spliterator() from sequential streams that do not have stateful ops. > > 2. It looks like StreamSpliterators.UnorderedSliceSpliterator.trySplit() > should disable splitting if (permitStatus == NO_MORE), instead of > (permits.get() == 0). As is, it would appear to unnecessarily disable > splitting after skipping, in the skip-only case. > > 3. It looks like SortedOps could override opEvaluateParallelLazy to no-op > in the already-sorted case, similar to DistinctOps in the already-distinct > case. > > 4. It looks like SliceOps.makeRef() could fuse adjacent skip()/limit() ops > (though I could see that being an overly niche optimization, especially if > it makes linked/consumed handling painful). > > 5. It looks like there is some dead code in StreamOpFlag? I don't see a > path where isPreserved() would return true in practice (it appears to only > be called in exactly 2 'minor optimization' places: SliceTask.doLeaf() and > DropWhileTask.doLeaf()). With the way StreamOpFlag.combineOpFlags() works, > it looks like in practice 0b00 is used to preserve, rather than 0b11. I > also don't see Type.OP, Type.TERMINAL_OP, or Type.UPSTREAM_TERMINAL_OP > being used anywhere. I assume some of the methods here are intended for > tests only (isStreamFlag(), canSet())... But given how much of this class > I'm not seeing being used, I'm wondering if I'm just missing something. > > Thank you, > Daniel >
