RFR: JDK-8199216: Quadratic layout time with nested nodes and pseudo-class in style sheet [v9]
Andy Goryachev
angorya at openjdk.org
Tue Sep 12 22:47:17 UTC 2023
On Thu, 7 Sep 2023 00:25:48 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:
>> This fix introduces immutable sets of `PseudoClass` almost everywhere, as they are rarely modified. These are re-used by caching them in a new class `ImmutablePseudoClassSetsCache`.
>>
>> In order to make this work, `BitSet` had to be cleaned up. It made assumptions about the collections it is given (which may no longer always be another `BitSet`). I also added the appropriate null checks to ensure there weren't any other bugs lurking.
>>
>> Then there was a severe bug in `toArray` in both the subclasses that implement `BitSet`.
>>
>> The bug in `toArray` was incorrect use of the variable `index` which was used for both advancing the pointer in the array to be generated, as well as for the index to the correct `long` in the `BitSet`. This must have resulted in other hard to reproduce problems when dealing with `Set<PseudoClass>` or `Set<StyleClass>` if directly or indirectly calling `toArray` (which is for example used by `List.of` and `Set.of`) -- I fixed this bug because I need to call `Set.copyOf` which uses `toArray` -- as the same bug was also present in `StyleClassSet`, I fixed it there as well.
>>
>> The net result of this change is that there are far fewer `PseudoClassState` objects created; the majority of these are never modified, and the few that are left are where you'd expect to see them modified.
>>
>> A test with 160 nested HBoxes which were given the hover state shows a 99.99% reduction in `PseudoClassState` instances and a 70% reduction in heap use (220 MB -> 68 MB), see the linked ticket for more details.
>>
>> Although the test case above was extreme, this change should have positive effects for most applications.
>
> John Hendrikx has updated the pull request incrementally with two additional commits since the last revision:
>
> - Use standard asserts in test
> - Add copyright header
🕶 looking good.
tested dynamic stylesheet loading, no ill effects.
-------------
Marked as reviewed by angorya (Reviewer).
PR Review: https://git.openjdk.org/jfx/pull/1076#pullrequestreview-1623311466
More information about the openjfx-dev
mailing list