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 <[email protected]> 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. >>
