RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

[Jeanette Winzenburg]

On Tue, 25 Feb 2020 14:06:23 GMT, dannygonzalez <github.com+6702882+dannygonzalez at openjdk.org> wrote:

>> I think that a starting point would be to decide on a spec for the listener notification order: is it specified by their registration order, or that is it unspecified, in which case we can take advantage of this for better performance. Leaving is unspecified and restricting ourselves to having it ordered is losing on both sides.
> 
> Even though the order of notifications is unspecified, the following tests fail if we use a HashMap vs LinkedHashMap i.e. the notifications are not in order of registration:
> 
> test.javafx.scene.control.TableViewTest > ensureSelectionIsCorrectWhenItemsChange FAILED
>     java.lang.AssertionError: expected:<0> but was:<-1>
>         at org.junit.Assert.fail(Assert.java:91)
>         at org.junit.Assert.failNotEquals(Assert.java:645)
>         at org.junit.Assert.assertEquals(Assert.java:126)
>         at org.junit.Assert.assertEquals(Assert.java:470)
>         at org.junit.Assert.assertEquals(Assert.java:454)
>         at test.javafx.scene.control.TableViewTest.ensureSelectionIsCorrectWhenItemsChange(TableViewTest.java:333)
> 
> test.javafx.scene.control.TreeTableViewTest > test_rt27180_expandBranch_laterSiblingSelected_singleSelection FAILED
>     java.lang.AssertionError: 
>         at org.junit.Assert.fail(Assert.java:91)
>         at org.junit.Assert.assertTrue(Assert.java:43)
>         at org.junit.Assert.assertTrue(Assert.java:54)
>         at test.javafx.scene.control.TreeTableViewTest.test_rt27180_expandBranch_laterSiblingSelected_singleSelection(TreeTableViewTest.java:2042)
> 
> test.javafx.scene.control.TreeViewTest > test_rt27185 FAILED
>     java.lang.AssertionError: expected:<TreeItem [ value: Mike Graham ]> but was:<null>
>         at org.junit.Assert.fail(Assert.java:91)
>         at org.junit.Assert.failNotEquals(Assert.java:645)
>         at org.junit.Assert.assertEquals(Assert.java:126)
>         at org.junit.Assert.assertEquals(Assert.java:145)
>         at test.javafx.scene.control.TreeViewTest.test_rt27185(TreeViewTest.java:603)
> 
> test.javafx.scene.control.TreeViewTest > test_rt27180_collapseBranch_laterSiblingAndChildrenSelected FAILED
>     java.lang.AssertionError: 
>         at org.junit.Assert.fail(Assert.java:91)
>         at org.junit.Assert.assertTrue(Assert.java:43)
>         at org.junit.Assert.assertTrue(Assert.java:54)
>         at test.javafx.scene.control.TreeViewTest.test_rt27180_collapseBranch_laterSiblingAndChildrenSelected(TreeViewTest.java:973)
> 
> test.javafx.scene.control.TreeViewTest > test_rt27180_expandBranch_laterSiblingSelected_singleSelection FAILED
>     java.lang.AssertionError: 
>         at org.junit.Assert.fail(Assert.java:91)
>         at org.junit.Assert.assertTrue(Assert.java:43)
>         at org.junit.Assert.assertTrue(Assert.java:54)
>         at test.javafx.scene.control.TreeViewTest.test_rt27180_expandBranch_laterSiblingSelected_singleSelection(TreeViewTest.java:992)
> 
> 
> These would need to be investigated to see why they assume this order.

Hmm .. personally, I would consider changing such a basic (and probably highly optimized) implementation used all over the framework is a very high risk change that at the worst outcome would detoriate internal and external code. So before going that lane, I would try - as you probably have, just me not seeing it :) - to tackle the problem from the other end:

> I know that in our application, the **thousands of listeners** unregistering when using a TableView was stalling the JavaFX thread.
> 

.. sounds like a lot. Any idea, where exactly they come into play?

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

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