Minor optimizations / questions about stream implementation

[Daniel Avery]

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.klang at 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-retn at openjdk.org> on behalf of
> Daniel Avery <danielaveryj at gmail.com>
> *Sent:* Friday, 10 January 2025 04:01
> *To:* core-libs-dev at openjdk.org <core-libs-dev at 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/core-libs-dev/attachments/20250117/54d7a5cf/attachment-0001.htm>