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 I was convinced, and updated the change to use List.of(). ------------- PR: https://git.openjdk.java.net/jdk/pull/1872
