On Wed, 12 Feb 2020 12:43:58 GMT, dannygonzalez <github.com+6702882+dannygonza...@openjdk.org> wrote:
>>> >>> Although that does seem odd behaviour to me. Obviously as the original >>> implementation was using an array I can see how the implementation drove >>> that specification. >>> >> >> whatever drove it (had been so since the beginning ot java desktop, at least >> since the days of swing), there is no way to change it, is it? >> >>> Non of the JavaFx unit tests test for that specific case as the unit tests >>> all passed. It would be nice if there was a specific test case for this >>> behaviour. >>> >> >> yeah, the test coverage is ... not optimal :) >> >>> I would need to store a registration count for each listener to satisfy >>> this requirement. >> >> a count plus some marker as to where it was added: >> >> addListener(firstL) >> addListener(secondL) >> addListener(firstL) >> >> must result in firstL.invalidated, seconL.invalidated, firstL.invalidated .. >> which brings us back to .. an array? > > The listeners are called back in the order they were registered in my > implementation although I didn’t see this requirement in the spec unless I > missed something. > > The performance unregistering thousands of listeners by TableView from the > array is killing the performance of our JavaFX application. It takes up to > 60% of JavaFX Application thread CPU and there are various bug reports around > this same TableView performance issue. > The set implementation has lowered this to below 1%. > > This pull request may be too fundamental a change to go into mainline. We > however have to build our local OpenJFX with this fix or our application is > unusable. > > I’m happy to receive any suggestions. > > Danny > > > On 12 Feb 2020, at 12:22, Jeanette Winzenburg > <notificati...@github.com<mailto:notificati...@github.com>> wrote: > > > Although that does seem odd behaviour to me. Obviously as the original > implementation was using an array I can see how the implementation drove that > specification. > > whatever drove it (had been so since the beginning ot java desktop, at least > since the days of swing), there is no way to change it, is it? > > Non of the JavaFx unit tests test for that specific case as the unit tests > all passed. It would be nice if there was a specific test case for this > behaviour. > > yeah, the test coverage is ... not optimal :) > > I would need to store a registration count for each listener to satisfy this > requirement. > > a count plus some marker as to where it was added: > > addListener(firstL) > addListener(secondL) > addListener(firstL) > > must result in firstL.invalidated, seconL.invalidated, firstL.invalidated .. > which brings us back to .. an array? > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on > GitHub<https://github.com/openjdk/jfx/pull/108?email_source=notifications&email_token=ABTEOIWBBLX3XA3JV23OP6LRCPSXRA5CNFSM4KQ7YBNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELQSLEY#issuecomment-585180563>, > or > unsubscribe<https://github.com/notifications/unsubscribe-auth/ABTEOIVSPJEJ6FAJ5SFT7V3RCPSXRANCNFSM4KQ7YBNA>. > > The listeners are called back in the order they were registered in my > implementation although I didn’t see this requirement in the spec unless I > missed something. yeah, you are right can't find that spec on sequence right now - maybe I dreamed it :) ------------- PR: https://git.openjdk.java.net/jfx/pull/108
