RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

[John Hendrikx]

On Tue, 10 Mar 2020 06:08:25 GMT, yosbits <github.com+7517141+yososs at openjdk.org> wrote:

>>> technically true - but the implementation was linear with a fixed sequence since-the-beginning-of-java-desktop-time
>>> (and sometimes, for good ol' beans properties, even exposed as api to access the array of listeners). So technically,
>>> we could go the path of explicitely spec'ing that the order is unspecified. Pretty sure that doing so and implementing
>>> it will break tons of application code that's subtly relying on fifo notification (f.i. register before or after skin
>>> has its own is a simple wide-spread trick) .. which all did it wrong and might deserve it ;-) But then if even internal
>>> code does it ..
>> 
>> So we can choose to specify it as ordered.
>> 
>> These are the 2 options I mentioned:
>> 1. Not specify the behavior and change the implementation in favor of performance. This could break applications as
>> you've mentioned. 2. Specify that the order is preserved and keep the ordered implementation behavior. This will allow
>> applications to rely on the behavior safely.
>> Right now we're losing on both sides. We keep a promise and we don't tell anyone about it. The only reason for this is
>> if we intend to change the behavior in the future, in which case we should add a doc comment saying that the order is
>> unspecified and is planned to change in the future, so there will be at least some warning.  Once we choose what to do
>> here it will make sense to continue to review the code with that decision in mind.
>
> In a minimal test I wrote (not a microbenchmark that removes listeners), I tried this PR code, but did not reproduce
> the performance improvement. I have attached a test program in my PR(#125)

@dannygonzalez You mentioned "There are multiple change listeners on a Node for example, so you can imagine with the
number of nodes in a TableView this is going to be a problem if the unregistering is slow.".

Your changes in this PR seem to be focused on improving performance of unregistering listeners when there are thousands
of listeners listening on changes or invalidations of the **same** property.  Is this actually the case?  Is there a
single property in TableView or TreeTableView where such a high amount of listeners are present?  If so, I think the
problem should be solved there.

As it is, I donot think this PR will help unregistration performance of listeners when the listeners are  spread out
over dozens of properties, and although high in total number, the total listeners per property would be low and their
listener lists would be short.  Performance of short lists (<50 entries) is almost unbeatable, so it is relevant for
this PR to know if there are any properties with long listener lists.

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

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