On Mon, 3 Apr 2023 07:11:07 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> 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. Couldn't you just add a fast path by calculating the hash code of the immutable set eagerly and storing it in a field? This entire method could probably be written to be allocation-free if the set passed into the method is already an immutable set. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1076#discussion_r1156550443
