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)
> ![image](https://github.com/openjdk/jfx/assets/66004280/2d80db3d-aca6-488e-a338-0759c82c35c8)

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

Reply via email to