TableView with many columns poor ui performance

[Danny Gonzalez]

Hi Kevin,

Thanks for your comments.

I am about to push some code for this. I have used a linkedHashMap which stores the listener and a count.
I have also updated the unit tests to test for the addListener and removeListener behaviour in the case you register the same listener more than once.

Would be interested in any comments or suggestions you may have.
This fix has allowed us to move forward to using Java 11 as otherwise the number of listeners unregistering when using a TableView was killing our application.

Thanks for your time.

Danny

> On 14 Feb 2020, at 14:32, Kevin Rushforth <kevin.rushforth at oracle.com> wrote:
> 
> Whether or not the specification of notifying a listener multiple times was intentional or driven by the implementation, it is part of the spec, and some applications could be relying on it. I do agree that there is no guarantee as to the order, which might simplify things. I can think of two possible implementations:
> 
> 1. Use a counter as you mentioned below as a possibility. If you did this, you would need to add an additional LinkedHashSet of objects containing the Listener and a count. You could initialize that set to null and only constructing it in the (rather unlikely) case that a listener is added when it is already in the set of listeners. You could then test for "if (countSet == null)" in the various operations and do the simple thing if it is.
> 
> 2. Since adding a listener more than once is unlikely, you could switch from a LinkedHashSet to an ArrayList the first time that a listener is added more than once. You would keep a Collection of listeners, initialize it to a LinkedHashSet, and dynamically switch from a LinkedHashSet to an ArrayList (copying existing entries) in addListener if the collection contains the listener already. This would be simpler logic than keeping track of the count for each object.
> 
> I would lean towards the second optoin. It gives the desired speed up for the common case where we don't add a listener more than once, and in the worst case, switches back to what we have today.
> 
> -- Kevin
> 
> 
> On 2/12/2020 4:03 AM, Danny Gonzalez wrote:
>> Hi Jeanette,
>> 
>> 
>> True, I hadn't read that spec in ObservableValueBase.
>> Although that does seem odd behaviour to me. Obviously as the original implementation was using an array I can see how the implementation drove that specification.
>> 
>> Non of the JavaFx unit tests test for that specific case as the unit tests all passed. It would be nice if there was a specific test case for this behaviour.
>> 
>> I would need to store a registration count for each listener to satisfy this requirement.
>> 
>> 
>> 
>> On 12 Feb 2020, at 11:34, Jeanette Winzenburg <fastegal at swingempire.de<mailto:fastegal at swingempire.de>> wrote:
>> 
>> 
>> Zitat von Danny Gonzalez <danny.gonzalez at screamingfrog.co.uk<mailto:danny.gonzalez at screamingfrog.co.uk>>:
>> 
>> Hi Ed,
>> 
>> I have submitted a pull request and the branch is here:
>> https://github.com/screamingfrog/jfx/tree/listeners_optimisation
>> 
>> 
>> 
>> hmm .. the change seems to be breaking spec of add/remove listeners
>> 
>> "If the same listener is added more than once, then it will be notified more than once".
>> 
>> or what am I missing?
>> 
>> 
>