On Tue, 12 Sep 2023 23:28:37 GMT, John Hendrikx <jhendr...@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 with a new target base due to a > merge or a rebase. The pull request now contains 19 commits: > > - Merge branch 'master' of https://git.openjdk.org/jfx into > feature/immutable-pseudoclassstate > - Use standard asserts in test > - Add copyright header > - Merge branch 'master' of https://git.openjdk.org/jfx into > feature/immutable-pseudoclassstate > - Merge remote-tracking branch 'upstream/master' into > feature/immutable-pseudoclassstate > - Avoid using Lambda in ImmutablePseudoClassSetsCache.of() > - Merge branch 'master' of https://git.openjdk.org/jfx into > feature/immutable-pseudoclassstate > - Fix another edge case in BitSet equals > > When arrays are not the same size, but there are no set bits in the ones > the other set doesn't have, two bit sets can still be considered equal > - Take element type into account for BitSet.equals() > - Base BitSet on AbstractSet to inherit correct equals/hashCode/toArray > > - Removed faulty toArray implementations in PseudoClassState and > StyleClassSet > - Added test that verifies equals/hashCode for PseudoClassState respect > Set contract now > - Made getBits package private so it can't be inherited > - ... and 9 more: https://git.openjdk.org/jfx/compare/624fe86f...962c43ea Hello! This might not be the right place to ask, so sorry about it in advance, I've found this pr after investigating a memory problem in an application with a lot of data in tables, I can see is available in openjfx 22 and later but still no LTS version above 21, are you planning to include this change in a future version of openjfx21? I find this a big improvement. Thank you! ------------- PR Comment: https://git.openjdk.org/jfx/pull/1076#issuecomment-2438587205
