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

Reply via email to