RFR: JDK-8297299 SequenceInputStream should not use Vector

[Jaikiran Pai]

On Sat, 19 Nov 2022 21:36:56 GMT, Markus KARG <duke at openjdk.org> wrote:

> There is no need to use a temporary Vector within the constructor of SynchronizedInputStream, as more efficient (non-synchronized) alternative code (like List.of) will do the same in possibly less time. While the optimization is not dramatic, it still makes sense to replace Vector unless synchronization is really needed.

src/java.base/share/classes/java/io/SequenceInputStream.java line 43:

> 41:  * on the last of the contained input streams.
> 42:  *
> 43:  * @author  Arthur van Hoff

Good catch. Looking at some other files in the JDK, your change here looks correct.

src/java.base/share/classes/java/io/SequenceInputStream.java line 83:

> 81:      */
> 82:     public SequenceInputStream(InputStream s1, InputStream s2) {
> 83:         e = Collections.enumeration(List.of(s1, s2));

Hello Markus, 
I think removing the `Vector` usage looks fine. The use of `List.of` in this constructor introduces a small change in behaviour of this constructor if the constructor is being passed a `null` `Inputstream`.

Previously, before this change, if the first `InputStream` parameter was null this constructor would throw a `NullPointerException` (because the call to `peekNextStream()` in this constructor throws that). However, if the second `InputStream` was `null` then the constructor would return back fine. Only later when the `SequenceInputStream` was used and it was time to move to use the second `InputStream`, it would throw the `NullPointerException` (from the same `peekNextStream()`).

Now, with the use of `List.of(s1, s2))`, the `NullPointerException` will get thrown early if either of these two parameters is `null`.

Whether or not this change in behaviour related to `NullPointerException` is acceptable (backed by a CSR) is something that others more familiar with this area would be able to tell. But I think it may not be desirable because depending on how the application might have been using the `SequenceInputStream` instance they may not have been encountering the `NullPointerException` (for the second param) if they never read the `SequenceInputStream` completely.


You could still remove the `Vector` usage from this code though, by constructing the collection differently instead of `List.of`.

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

PR: https://git.openjdk.org/jdk/pull/11249