RFR: JDK-8199216: Quadratic layout time with nested nodes and pseudo-class in style sheet [v8]

[Jose Pereda]

On Tue, 15 Aug 2023 14:08:08 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> modules/javafx.graphics/src/main/java/com/sun/javafx/css/PseudoClassState.java line 65:
>> 
>>> 63:     /** {@inheritDoc} */
>>> 64:     @Override
>>> 65:     public <T> T[] toArray(T[] a) {
>> 
>> You have mentioned that this was a bug but hard to reproduce.
>> Could you add a test that somehow calls `toArray` that fails before your patch and passes now?
>
> It's definitely a bug, but it would only appear if you had bit sets which required more than 1 long to store their data **and** you called `toArray`.  It would then throw an `ArrayIndexOutOfBoundsException` in this line:
> 
>     final long state = getBits()[index];
> 
> This is because `index` is used for two unrelated purposes:
> - the index into the array of longs
> - the index of the entry in the resulting array
> 
> If the outer loop ever needs two passes, it again wants to read a `long` using `getBits()[index]` but as `index` was advanced (multiple times) while filling entries in the result array, it won't be the correct index at all.
> 
> Anyway, I'm not sure what the value would be of a test.  Since `toArray` is no longer implemented by us but by `AbstractSet`, I'd just be testing the `AbstractSet` code.

Okay, thanks for the clarification. 

So if I get it right, the removal of `toArray` doesn't have to do with the bug, but with the fact that with the immutable set it is not longer required?

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

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