On Thu, 11 Jan 2024 09:33:09 GMT, Marius Hanl <mh...@openjdk.org> wrote:
>> John Hendrikx has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains eight additional >> commits since the last revision: >> >> - Merge branch 'master' into feature/selector-performance-improvement >> - Add since tag to new API >> - Add deprecated annotation and fixed deprecation descriptions >> - Use copyOf instead of Collections.unmodifiableList(new ArrayList<>(x)) >> - Pull up frozen field to abstract FixedCapacitySet class >> - Add copyright header >> - Deprecate StyleClass and remove StyleClassSet for faster solution >> - Fix regression > > modules/javafx.graphics/src/main/java/com/sun/javafx/css/FixedCapacitySet.java > line 94: > >> 92: return maximumCapacity == 0 ? empty() >> 93: : maximumCapacity == 1 ? new Single<>() >> 94: : maximumCapacity == 2 ? new Duo<>() > > I can confirm that most of the code in my application uses 1-2 styleclasses. > Since all controls have one styleclass by default (some even more, think > about `TextInputControl` with `text-input` -> `TextField` with `text-field`), > does it make sense to also implement a specialized `Triple` class? > > We also often add 2 more styleclasses, so 3 style classes seems like a > usecase that happens quite often. > I also can confirm that 4 styleclasses or even more are very rare. > I only found one: (`.label` + 3 added from us) >  Thanks for having a look, it's good to have some more use cases from other applications! The sets are used both for the style classes on `Node`s but also the set of style classes in CSS selectors (multiple style classes in selectors are even more rare I think than multiple style classes on `Node`s). I think from a performance perspective, `Duo` and `Hashless` are likely very closely matched, but I can do some testing in that area. The reason I say this is that I only got a very minor boost from including `Duo`, but kept it because it is a bit more memory efficient. If you want, I'm also curious about how many styles are used in your application (you can see this by looking at the size of `StyleClassSet#styleClasses` after all styles are loaded). ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1316#discussion_r1449480442