On Fri, 1 Dec 2023 15:32:32 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> I wouldn't do any of the above unless there is a very good reason (and I'm >> not seeing one). Just use standard `List.of` as the last step (or use the >> `Collections.unmodifableList` wrapper); you'll get the most optimized, >> automatically maintained, bugfree, immutable `List` implementation Java has >> to offer. It means another copy will be made; that's fine, this only >> happens once -- it's not in a hot path. >> >> If you feel like optimizing something, don't bother with >> `Control.getCssMetaData` either; instead, deduplicate the property lists so >> there is only one list per unique `Skin` + `Control` combo. That saves a >> **complete** list per control **instance**. > > Good point, though I would still not use List.of() because of the unnecessary > copy. > I agree on `Skin` + `Control` copy. Just not sure how... a static hash > table perhaps? I will approve this as is, but I agree with John that it would be better to use one of the existing implementations of List: `new UnmodifiableArrayList`, `List.of`, or `Collections.unmodifiableList`. The cost of the extra copy for `List.of` is in the noise compared to the benefit of not having to maintain yet another special case List class. Similarly, the extra word of data storage in `UnmodifiableArrayList` is in the noise. This is a one-per-class not one-per instance list. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1293#discussion_r1412400728
