RFR-8148838: Stream.flatMap(...).spliterator() cannot properly split after tryAdvance()
Tagir F. Valeev
amaembo at gmail.com
Wed Feb 3 12:19:45 UTC 2016
Here's updated webrev:
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,
>> On 2 Feb 2016, at 09:24, Tagir F. Valeev <amaembo at gmail.com> wrote:
>> Please review and sponsor this fix:
>> When buffer traversal is already started, and split is requested, then
>> the existing buffer should be carefully transferred to the prefix
>> 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.
More information about the core-libs-dev