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