RFR: 8281298: Revise the creation of unmodifiable list

Claes Redestad redestad at openjdk.java.net
Sun Feb 6 22:14:11 UTC 2022


On Sat, 5 Feb 2022 20:29:50 GMT, Xue-Lei Andrew Fan <xuelei at openjdk.org> wrote:

> In [JDK-8281289](https://bugs.openjdk.java.net/browse/JDK-8281289), the SSLParameters class was updated with the patch:
> 
> - return Collections.<SNIServerName>unmodifiableList(new ArrayList<>(sniNames.values()));
> + return List.copyOf(sniNames.values());
> 
> However, there's a small compatibility risk with this change, `List.copyOf(...).contains(null)` will throw NPE while `Collections.unmodifiableList(...).contains(null)` won't. It may be not worthy of the risk although the impact may be minimal.
> 
> This update will re-use the Collections.unmodifiableList() methods, but re-org the code for better performance.

This looks like an appropriate solution which avoids the minor compatibility risk introduced by the previous change - and you might even end up being more efficient both when setting and reading the names/matchers. 

(Since we're going down the route of optimizing this: the `type` of a `SNIServerName` is constrained to a value between 0 and 255, so there's likely a small micro-optimization opportunity in using a `BitSet` for the duplicate checking rather a `ArrayList<Integer>`.. A `BitSet(256)` should have lower memory use and the existence check will run in O(1) instead of O(n) time. Might not be worthwhile to optimize these setters, though..)

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

Marked as reviewed by redestad (Reviewer).

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



More information about the security-dev mailing list