On Mon, 28 Dec 2020 21:25:57 GMT, Сергей Цыпанов 
<[email protected]> wrote:

>>> What about using List.of() instead?
>> 
>> For now, the Collections.singletonList() is more compact, which uses one 
>> class variable.  While List.of(T) shares the internal implementation with 
>> List.of(T t1, T t2), which uses two class variables.
>
>> > What about using List.of() instead?
>> 
>> For now, the Collections.singletonList() is more compact, which uses one 
>> class variable. While List.of(T) shares the internal implementation with 
>> List.of(T t1, T t2), which uses two class variables.
> 
> There's one more issue about it: `List.of()` throws NPE when argument is 
> `null` while `Collections.singletonList()` returns a list with `null` inside.

As @stsypanov points out the null-hostile behavior of the collection returned 
from `List.of()` can pose a compatibility issue: you'd have to guard against 
null inputs, and if the lists leak through public API then observable behavior 
might change in ways that are prohibitive. For the code in question it seem the 
lists are only created and passed around temporarily, mainly iterating over 
them. And it appears surrounding could would already throw NPEs if the values 
wrapped were ever null. So using `List.of` should be fine here.

As for the impl class for a single element `List.of()` having two fields then 
that should be footprint neutral since the second field will fit into an 
alignment gap. There should be no footprint advantage of using 
`Collections.singletonList`, which was verified by JOL[0] and by checking 
allocation pressure in microbenchmarks. IIRC this was the case on both on 
64-bit and 32-bit VMs.

I favor `List.of` over `Collections` variants for making code more readable 
first. They _also_ make it easier to refactor (adding/removing items) without 
risking worse performance, which can more easily happen when mixing  
`Collections.unmodifiable`, `.empty` and `singleton` (see: bi- and megamorphic 
call sites).

The code changes in this PR probably might make some difference from removing 
the need for a varargs array. But if the goal is to reduce allocation pressure 
of the methods involved there seem to be bigger fish to fry - such as the use 
of `LinkedList` in `KeyShareExtension`, and the nested for loops in 
`SSLConfiguration.getEnabledExtensions`. Are there any targeted microbenchmarks 
for these operations or do these operations have some weight in any of the 
existing micros?

[0] https://github.com/openjdk/jol

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

PR: https://git.openjdk.java.net/jdk/pull/1872

Reply via email to