Re: RFR: 8258852: Arrays.asList() for single item could be replaced with Collections.singletonList()
On Mon, 28 Dec 2020 21:25:57 GMT, Сергей Цыпанов 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
Re: RFR: 8258852: Arrays.asList() for single item could be replaced with Collections.singletonList()
On Thu, 24 Dec 2020 17:30:41 GMT, Xue-Lei Andrew Fan 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. There's one more issue about it: `List.of()` throws NPE when argument is `null` while `Collections.singletonList()` returns a list with `null` inside. - PR: https://git.openjdk.java.net/jdk/pull/1872
Re: RFR: 8258852: Arrays.asList() for single item could be replaced with Collections.singletonList()
On Thu, 24 Dec 2020 15:00:04 GMT, Sean Mullan 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. - PR: https://git.openjdk.java.net/jdk/pull/1872
Re: RFR: 8258852: Arrays.asList() for single item could be replaced with Collections.singletonList()
On Wed, 23 Dec 2020 00:56:25 GMT, Xue-Lei Andrew Fan wrote: > If there is only one item, the call to Arrays.asList() could be replaced with > Collections.singletonList() for less memory occupation. This update also > includes some other code cleanup, like redundant variables in the related > files. > > Code cleanup only, no new regression test. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8258852 What about using List.of() instead? - PR: https://git.openjdk.java.net/jdk/pull/1872
RFR: 8258852: Arrays.asList() for single item could be replaced with Collections.singletonList()
If there is only one item, the call to Arrays.asList() could be replaced with Collections.singletonList() for less memory occupation. This update also includes some other code cleanup, like redundant variables in the related files. Code cleanup only, no new regression test. Bug: https://bugs.openjdk.java.net/browse/JDK-8258852 - Commit messages: - 8258852: Arrays.asList() for single item could be replaced with Collections.singletonList() Changes: https://git.openjdk.java.net/jdk/pull/1872/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1872&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8258852 Stats: 16 lines in 3 files changed: 0 ins; 5 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/1872.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1872/head:pull/1872 PR: https://git.openjdk.java.net/jdk/pull/1872
