RFR: JDK-8199216: Memory leak and quadratic layout time with nested nodes (hbox) and pseudo-class in style sheet [v3]

[John Hendrikx]

On Mon, 3 Apr 2023 06:22:39 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> John Hendrikx has updated the pull request incrementally with three additional commits since the last revision:
>> 
>>  - Base BitSet on AbstractSet to inherit correct equals/hashCode/toArray
>>    
>>    - Removed faulty toArray implementations in PseudoClassState and
>>    StyleClassSet
>>    - Added test that verifies equals/hashCode for PseudoClassState respect
>>    Set contract now
>>    - Made getBits package private so it can't be inherited
>>  - Remove unused code
>>  - Ensure Match doesn't allow modification
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/css/ImmutablePseudoClassSetsCache.java line 53:
> 
>> 51:         return CACHE.computeIfAbsent(
>> 52:             Objects.requireNonNull(pseudoClasses, "pseudoClasses cannot be null"),
>> 53:             k -> Set.copyOf(pseudoClasses)
> 
> The call to `computeIfAbsent` will always do the following things:
> 1. Capture the `pseudoClasses` argument
> 2. Call `Map.get` internally
> 3. ...which calls `Set.hashCode`
> 4. ...which, for `AbstractSet.hashCode`, iterates over the entire set using an iterator and calls `hashCode` for each element.
> 
> Since there is no fast path, I wonder whether this may have negative performance implications for larger scene graphs.

I don't think there is much of an impact, scene graph size should not affect it.  The sets involved are tiny, usually consisting of 1 or 2 items and very rarely 3 or possibly more(*). The hash code of `PseudoClass` is very fast, as they're singletons which don't override their hash code.

(*) The sets are based on pseudo class combinations encountered in a Scene that would have an active role in changing the control's visual appearance.  A pseudo class that doesn't affect appearance is filtered.  This means usually only states like `hovered`, `focused` or `armed` have an affect.  A class like `focused-within` would only be part of the set if the control involved has styles based on that class that would affect its appearance.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1076#discussion_r1155561513