On Wed, 14 Apr 2021 12:33:23 GMT, dannygonzalez <github.com+6702882+dannygonza...@openjdk.org> wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8185886 >> >> Optimisation to ExpressionHelper.Generic class to use Sets rather than >> Arrays to improve listener removal speed. >> >> This was due to the removal of listeners in TableView taking up to 50% of >> CPU time on the JavaFX Application thread when scrolling/adding rows to >> TableViews. >> >> This may alleviate some of the issues seen here: >> >> TableView has a horrific performance with many columns #409 >> https://github.com/javafxports/openjdk-jfx/issues/409#event-2206515033 >> >> JDK-8088394 : Huge memory consumption in TableView with too many columns >> JDK-8166956: JavaFX TreeTableView slow scroll performance >> JDK-8185887: TableRowSkinBase fails to correctly virtualise cells in >> horizontal direction >> >> OpenJFX mailing list thread: TableView slow vertical scrolling with 300+ >> columns >> https://mail.openjdk.java.net/pipermail/openjfx-dev/2020-January/024780.html > > dannygonzalez has updated the pull request incrementally. I think it is also important to check if this is still an issue. A PR which reduces the amount of listeners on Scene/Window was integrated in JavaFX 17: https://github.com/openjdk/jfx/pull/185 Are you experiencing problems still under 17? I think PR 108 is addressing symptoms of a problem but not the root cause -- is it ever useful to have >10000 listeners on a single property which would warrant the use of Sets? PR 185 solved this by not registering a listener for each Node on the Scene and Window, and there was some confirmation from the OP that this also addressed the issue, see here: https://github.com/openjdk/jfx/pull/108#issuecomment-615125904 --John On 01/10/2021 13:27, John Nader wrote: > This change is very near completion. According to @yososs > <https://github.com/yososs>, what remains is to split the work into two > separate PRs. The first PR must contain the updated test covering > additional cases to ensure backwards compatibility. The second should > contain the performance enhancement changes. This will allow the tests > to be merged into master first, to ensure the behavior is maintained > before and after the performance enhancement. > > I see that @dannygonzalez <https://github.com/dannygonzalez> has not > been able to continue this work. If possible and appropriate, I would > like to finish this work. I have already setup a build environment and > have verified that the updated tests alone pass when integrated with the > master branch and have 100% coverage of the modified class. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <https://github.com/openjdk/jfx/pull/108#issuecomment-932146935>, or > unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAHTETIDTFZAQSKBVUFO3BLUEWLI7ANCNFSM4KQ7YBNA>. > Triage notifications on the go with GitHub Mobile for iOS > <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> > or Android > <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. > > ------------- PR: https://git.openjdk.java.net/jfx/pull/108
