RFR: JDK-8277095 : Empty streams create too many objects

[kabutz]

On Mon, 15 Nov 2021 18:08:22 GMT, Philippe Marschall <duke at openjdk.java.net> wrote:

> I have a similar project at [empty-streams](https://github.com/marschall/empty-streams). A couple of notes:
> 
> 1. I found the need for streams to be stateful. I had need for the following state:
>    
>    1. closed
>    2. ordered
>    3. parallel
>    4. sorted

I found the need for ALL the Spliterator characteristics, plus some others that I interleaved between the Spliterator bits. That way I could fit them into a single int.

Parallel was too tricky though, so in that case I return a new stream built with the standard StreamSupport(spliterator, true).

>    5. closeHandler

Similarly when someone calls closeHandler(), I also return a new stream built with StreamSupport.

>    6. comparator (on EmptyStream)

Yes, we need this for TreeSet and ConcurrentSkipListSet. I was hoping to avoid it, but there is no way around if we want to be completely correct.

>       A shared instance can not be used because of `#close`.

Agreed. I fixed that.

> 2. I have a `PrimitiveIterator` that short circuits `#next` and `#forEachRemaining` as well.

Oh that's a good suggestion - thanks.
> 3. I made many methods just return `this` after checking for operated on and closed:
>    
>    1. `#filter` `#map`, `#flatMap`, `#mapMulti`, `#distinct`, `#peek`, `#limit`, `#skip`, `#dropWhile`, `#takeWhile`.

I now always return a new EmptyStream. Fortunately escape analysis gets rid of a lot of objects.

>    2. These do a state change state as well `#sequential`, `#parallel`, `#unordered`, `#sorted`, `#onClose`.

unordered() only does a state change if it currently ORDERED, otherwise it is correct to return this. The logic is convoluted and weird though. I have a more thorough test case that I need to incorporate into my EmptyStreamTest and which tests all the JDK collections, including the wrapper and immutable collections.

> 4. I made my `EmptyBaseStream` implement `BaseStream` and make `EmptyIntLongDoubleStream` extend from this class as `IntLongDoubleStream` all extend `BaseStream`. This allowed me to move the following methods up in the hierarchy `#isParallel` , `#onClose`, `#sequential`, `#parallel`, `#unordered`.

Interesting idea. I'm not sure if that would work for me. I will have a look if I can also do this, but I doubt it. A lot of the methods, especially the primitive ones, are repeats of each other. It might be really nice to put that into a common superclass. 

> 5. Is there any reason why you make `#parallel` "bail out" to `StreamSupport` rather than just do a state change?

I tried to keep the characteristics identical to what we had before and parallel seemed particularly challenging to get right. I might attempt this again at a later stage, but for now I want to keep it like it is. I don't think parallel() is the most common use case, except with very large collections perhaps. Even though I thought I was careful with the characteristics, I still have a few edge cases I need to iron out.

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

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