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)  ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1316#discussion_r1448557802