RFR: 8252936: Optimize removal of listeners from ExpressionHelper.Generic [v3]

[Kevin Rushforth]

On Fri, 1 Oct 2021 21:11:01 GMT, John Nader <github.com+1619657+nadernader99 at openjdk.org> wrote:

> Here is the minimal application I am using to recreate the issue.

I'll attach the test case you provided to the bug report.

> The question now is should the work done on this PR be abandoned. It addresses a current performance limitation taking complexity from O(n log n) to O(1).

That is a fair question, but it begs two additional questions. Are there enough real world examples where performance is being hurt by using an ArrayList to justify the effort and risk (more on that below)? If so, are there other solutions that would reduce the number of listeners needed? The original problems that motivated this fix were largely addressed by [JDK-8252935](https://bugs.openjdk.java.net/browse/JDK-8252935) / PR #185 and [JDK-8252811](https://bugs.openjdk.java.net/browse/JDK-8252811) / PR #298.

> It appears to be well isolated with low risk to backwards compatibility. The reason work was stopped was a concern that the tests should go first in a separate PR.

I disagree that this is a low risk change. This proposed fix touches a fundamental area used by all bindings, and does so in a way that will increase memory usage -- and might negatively impact performance -- in the common case of a small number of listeners. By unconditionally switching from an `ArrayList` to a `HashSet`, the memory required is significantly increased, which will be most noticeable for the common case of only a few listeners. An adaptive solution, which starts out using an `ArrayList` and dynamically switches to a `LinkedHashSet` (which preserves notification order...I know it isn't specified, but changing that is likely to break some applications) when crossing some threshold of the number of listeners, could mitigate this concern at the cost of added complexity.

If you still want to pursue this, you will need to do a fair amount of work to provide the additional tests that will be needed, to motivate the need for this enhancement (given that the original reasons are mostly addressed), and to resolve the issues that were raised in this PR. The mechanics of doing this are pretty easy. First, read the [CONTRIBUTING guideline](https://github.com/openjdk/jfx/blob/master/CONTRIBUTING.md), specifically the part about signing the OCA, and create a new PR starting from this one. You would then add the author of this PR as a contributor.

In the mean time, I'm going to move this PR to Draft (which I should have done long ago), since it stalled and not being actively worked on or reviewed.

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

PR: https://git.openjdk.java.net/jfx/pull/108