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

John Hendrikx jhendrikx at openjdk.org
Thu Mar 30 17:19:28 UTC 2023


On Thu, 30 Mar 2023 13:01:09 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

> One more optimization we could have a look at is too create the `BitSet` or related subclasses with a predefined size, so we don't need to grow the `long[] bits` array too often. `PseudoClassState` already has a constructor that accepts another collection, but can not use this information to create a bigger `bits` array. (Like e.g. the `ArrayList` or `HashSet` constructor)

I only looked at `PseudoClassState`, and it will rarely exceed the 64 bits that are available, although users can of course create as many pseudo classes as they want -- I wouldn't recommend this though with how the CSS caching logic is currently implemented.  A new key is created for every combination of pseudo classes encountered per depth level in a live scenegraph (but only for classes that can affect styling).

The low amount of pseudo classes is also why I didn't bother creating an immutable variant which maps them to bits.  This is IMHO an optimization that was done because of the mutable nature of `PseudoClassState` (resulting in many many copies), but which is mostly irrelevant now that most of them are immutable.

Perhaps this is useful for the other subclass `StyleClassSet`, but I have my doubts it is currently a pressing performance issue.

> modules/javafx.graphics/src/main/java/com/sun/javafx/css/BitSet.java line 227:
> 
>> 225:         final T t = cast(o);
>> 226: 
>> 227:         if (t == null) {  // if cast failed, it can't be part of this set
> 
> If I understand the code correctly, this can't happen right now, right?

`BitSet` did not conform to the collection contract here.  It can happen in current JavaFX releases by calling a method that returns `Set<PseudoClass>` and asking it `contains("text")` -- as `String` cannot be cast to `PseudoClass`, this will result in a `ClassCastException` while it should just return `false`.

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

PR Comment: https://git.openjdk.org/jfx/pull/1076#issuecomment-1490316678
PR Review Comment: https://git.openjdk.org/jfx/pull/1076#discussion_r1153280950


More information about the openjfx-dev mailing list