On Tue, 25 Feb 2020 14:06:23 GMT, dannygonzalez <github.com+6702882+dannygonza...@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