On Fri, 5 Jan 2024 01:45:56 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:

>> Improves performance of selector matching in the CSS subsystem. This is done 
>> by using custom set implementation which are highly optimized for the most 
>> common cases where the number of selectors is small (most commonly 1 or 2). 
>> It also should be more memory efficient for medium sized and large 
>> applications which have many style names defined in various CSS files.
>> 
>> Due to the optimization, the concept of `StyleClass`, which was only 
>> introduced to assign a fixed bit index for each unique style class name 
>> encountered, is no longer needed. This is because style classes are no 
>> longer stored in a `BitSet` which required a fixed index per encountered 
>> style class.
>> 
>> The performance improvements are the result of several factors:
>> - Less memory use when only very few style class names are used in selectors 
>> and styles from a large pool of potential styles (a `BitSet` for potentially 
>> 1000 different style names needed 1000 bits (worst case)  as it was not 
>> sparse).
>> - Specialized sets for small number of elements (0, 1, 2, 3-9 and 10+)
>> - Specialized sets are append only (reduces code paths) and can be made read 
>> only without requiring a wrapper
>> - Iterator creation is avoided when doing `containsAll` check thanks to the 
>> inverse function `isSuperSetOf`
>> - Avoids making a copy of the list of style class names to compare (to 
>> convert them to `StyleClass` and put them into a `Set`) -- this copy could 
>> not be cached and was always discarded immediately after...
>> 
>> The overall performance was tested using the JFXCentral application which 
>> displays about 800 nodes on its start page with about 1000 styles in various 
>> style sheets (and which allows to refresh this page easily).  
>> 
>> On JavaFX 20, the fastest refresh speed was 121 ms on my machine.  With the 
>> improvements in this PR, the fastest refresh had become 89 ms.  The speed 
>> improvement is the result of a 30% faster `Scene#doCSSPass`, which takes up 
>> the bulk of the time to refresh the JFXCentral main page (about 100 ms 
>> before vs 70 ms after the change).
>
> 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)

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1316#discussion_r1448557802

Reply via email to