On Sat, 19 Nov 2022 21:36:56 GMT, Markus KARG <d...@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