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

[John Hendrikx]

On Tue, 15 Aug 2023 22:21:15 GMT, Jose Pereda <jpereda at openjdk.org> wrote:

>> I removed it because my fix requires that `toArray` works correctly.  The easiest way to get a correctly working version is to extend `AbstractSet`, which provides a default implementation that works correctly.  As I think the default implementation is good enough and performs well enough, I saw no reason to fix the broken version.
>
> The original ("broken") version has been working fine, and no bugs have been reported so far, and there would be a reason to have a custom implementation instead of the one in `AbstractSet` in the first place. 
> 
> I'm not against removing it, but only after we are certain that this implementation is no longer needed. 
> 
> Also, have you tried fixing it instead of removing it? If you have, are there any differences when you run the test with one or the other?

You could check this yourself if you want.  The `BitSet` class had problems in many places, was largely untested and, to be very honest, should never have passed code review.  It violated the `Set` contract almost everywhere, which is problematic since sets backed by this `BitSet` class were exposed in several places where they could be accessed by users.

A simple test shows the problem (it doesn't throw an exception though, I misread that):

    public static void main(String[] args) {
        for (int i = 0; i < 65; i++) {
            PseudoClassState.getPseudoClass("" + i);
        }

        System.out.println(Arrays.asList(new PseudoClassState(List.of("0", "1", "64")).toArray()));
    }

Prints:

    [0, null, null]

There is no point in fixing the existing code; it won't perform any better, but would require writing additional test cases to verify that an implementation we don't need is doing what we can get for free.

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

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