RFR: JDK-8322964 Optimize performance of CSS selector matching [v2]

John Hendrikx jhendrikx at openjdk.org
Thu Jan 11 22:39:38 UTC 2024


On Thu, 11 Jan 2024 09:33:09 GMT, Marius Hanl <mhanl at 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


More information about the openjfx-dev mailing list