RFR: 8185886: Improve scrolling performance of TableView and TreeTableView

[dannygonzalez]

On Tue, 25 Feb 2020 13:55:50 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

>> Replying to @nlisker and @kevinrushforth general comments about the memory consumption of using a LinkedHashMap:
>> 
>> I agree that at the very least these should be lazy initialised when they are needed and that we should initially allocate a small capacity and load factor.
>> 
>> We could also have some sort of strategy where we could use arrays or lists if the number of listeners are less than some threshold (i.e. keeping the implementation with a similar overhead to the original) and then switch to the HashMap when it exceeds this threshold. This would make the code more complicated and I wonder if this is the worth the extra complexity.
>>  
>> I know that in our application, the thousands of listeners unregistering when using a TableView was stalling the JavaFX thread. 
>> 
>> I am happy to submit code that lazy initialises and minimises initial capacity and load factor as suggested or happy to take direction and suggestions from anyone with a deeper understanding of the code than myself.
> 
> 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.

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

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