On Tue, 25 Feb 2020 13:55:50 GMT, Nir Lisker <nlis...@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
