On Thu, 30 Mar 2023 13:01:09 GMT, Marius Hanl <mh...@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
