RFR-8148838: Stream.flatMap(...).spliterator() cannot properly split after tryAdvance()

[Tagir F. Valeev]

Hello!

PS> I still disagree and pushing back on the support for splitting
PS> after partial traversal. It’s not a pattern i think we should
PS> encourage and propagate so such behaviour can be generally relied
PS> upon, and predominantly for edge cases. That’s where the
PS> complexity string is being pulled on. While your fix in isolation
PS> is not terribly complex, it is more complex than the alternative
PS> (which was actually the intent of the current impl, we just forget to include the check).

I still don't like doing this, but as Brian agreed with you [1], seems
I have no other choice. Here's updated webrev:
http://cr.openjdk.java.net/~tvaleev/webrev/8148838/r3/

With best regards,
Tagir Valeev.

[1] https://twitter.com/BrianGoetz/status/694985109628387328

PS> Paul.

>> On 3 Feb 2016, at 12:19, Tagir F. Valeev <amaembo at gmail.com> wrote:
>> 
>> Hello!
>> 
>> Here's updated webrev:
>> http://cr.openjdk.java.net/~tvaleev/webrev/8148838/r2/
>> 
>> PS> My inclination is to keep this simple and not support splitting after partial traversal.
>> 
>> PS> Sometimes splitting after partial traversal might be more complex
>> PS> not support (e.g. ArrayList). In other cases it’s more complex to
>> PS> support and in such cases i would argue it is not worth it since
>> PS> this kind of functionality is an edge case.
>> 
>> I would still like that this problem is fixed (i.e. support splitting
>> after advance, not just return null). This is probably an edge case
>> for JDK, but might be crucial for library writers. As a library writer
>> I actually had bad times working around this issue. While returning
>> null instead of incorrect splitting would indeed be helpful, it will
>> unnecessarily remove parallelization capabilities in some cases. For
>> example, sometimes it's desired to extract the first element from the
>> stream and create the stream from the tail. Returning null here would
>> mean that the resulting stream will never split after that.
>> 
>> To my opinion this fix is not very complex. It just adds several lines
>> into single method (trySplit()). I added a comment which clarifies the
>> intention. Other changes like extraction of initPusher() do not add
>> complexity. Actually they reduce the complexity as four identical
>> lambdas are merged into one. This patch actually reduces the size of
>> created class files. With javac 9-ea+99 StreamSpliterators.java is
>> compiled into 79004 bytes after my patch and 79472 bytes before my
>> patch. Also this patch does not require primitive specializations and
>> adds no overhead for common case when traversal is not started.
>> 
>> PS> Testing wise you are right to be concerned about the increase in
>> PS> test execution time. The lack of flatMap is definitely an
>> PS> omission. In this case I think it is ok to replace map with
>> PS> flatMap, as both result in stuff going into the holding buffer,
>> PS> and we can use the smaller data sets, e.g.
>> PS> "StreamTestData<Integer>.small”, that were recently added.
>> 
>> I replaced all the datasets with .small alternatives (I think it's ok
>> here as WrappingSpliterator behavior does not differ much for various
>> sources)  and  removed all .map tests. Now the test works faster
>> (like 25 sec -> 17 sec on by box). Please check.
>> 
>> With best regards,
>> Tagir Valeev.
>> 
>> PS.
>> 
>> PS> Paul.
>> 
>>>> On 2 Feb 2016, at 09:24, Tagir F. Valeev <amaembo at gmail.com> wrote:
>>>> 
>>>> Please review and sponsor this fix:
>>>> 
>>>> https://bugs.openjdk.java.net/browse/JDK-8148838
>>>> http://cr.openjdk.java.net/~tvaleev/webrev/8148838/r1/
>>>> 
>>>> When buffer traversal is already started, and split is requested, then
>>>> the existing buffer should be carefully transferred to the prefix
>>>> part.
>>>> 
>>>> The only problem I see here is the testing time. Due to
>>>> permuteFunctions() call it increases significantly what I added the
>>>> flatMap() test. If this becomes an issue, I can write new simple test
>>>> which tests flatMap() only (without permutations with other
>>>> intermediate operations).
>>>> 
>>>> With best regards,
>>>> Tagir Valeev.
>>>> 
>>