On Thu, 3 Sep 2020 07:44:55 GMT, yosbits
wrote:
>>>
>>>
>>> When the startup time is measured by eye, the impression changes depending
>>> on the individual difference.
>>
>> my eye is a precision instrument :) Seriously, who would do such a thingy?
>> Obviously, it must be measured, for ze
On Thu, 27 Aug 2020 00:07:21 GMT, Kevin Rushforth wrote:
>> So far, there are two alternative fixes for the bad performance issue while
>> scrolling TableView/TreeTableViews:
>>
>> - this one #108, that tries to improve the performance of excessive number
>> of `removeListener` calls
>> - the
On Wed, 2 Sep 2020 10:46:48 GMT, Jeanette Winzenburg
wrote:
>> When the startup time is measured by eye, the impression changes depending
>> on the individual difference.
>> The effect of runLater affects your experience.
>>
>> However, I succeeded in further improving performance by eliminati
> If there are many columns, the current TableView will stall scrolling.
> Resolving this performance issue requires column
> virtualization. Virtualization mode is enabled when the row height is fixed
> by the following method.
> `tableView.setFixedCellSize(height)`
>
> This proposal includes a
On Tue, 1 Sep 2020 20:57:58 GMT, yosbits
wrote:
>
>
> When the startup time is measured by eye, the impression changes depending on
> the individual difference.
my eye is a precision instrument :) Seriously, who would do such a thingy?
Obviously, it must be measured, for zeroth
approximatio
On Tue, 1 Sep 2020 14:12:15 GMT, Jeanette Winzenburg
wrote:
>> Since there was a problem that the cell was not updated when the window was
>> maximized, I added a fix to
>> TreeTableViewSkin.java.
>> Added test code (BigTreeTableViewTest) for TreeTableView to the first post.
>>
>> @kleopatra
>
On Mon, 31 Aug 2020 19:16:31 GMT, yosbits
wrote:
>
>
> Since there was a problem that the cell was not updated when the window was
> maximized, I added a fix to
> TreeTableViewSkin.java.
that's just the added listener to hbar properties (same as in TableViewSkin),
or anything else changed?
On Sun, 30 Aug 2020 13:20:49 GMT, Jeanette Winzenburg
wrote:
>> When setting the cell height to a fixed value with setFixedCellSize(),
>> column virtualization can improve performance.
>>
>> As the next step, I think it is possible to try to enable column
>> virtualization regardless of wheth
> If there are many columns, the current TableView will stall scrolling.
> Resolving this performance issue requires column
> virtualization. Virtualization mode is enabled when the row height is fixed
> by the following method.
> `tableView.setFixedCellSize(height)`
>
> This proposal includes a
On Thu, 27 Aug 2020 06:37:53 GMT, yosbits
wrote:
>> 19fabf2eafcb02b519d39a1b0a9dad5b9209db64
>>
>> * Constructor creates multiple cell nodes, but the
>> Fixed a wasteful problem of creating all cells and deleting out-of-display
>> cells because the fixedCellSize attribute
>> was not initialize
On Sat, 18 Apr 2020 17:40:30 GMT, yosbits
wrote:
>> My name is "Naohiro Yoshimoto"
>> I have received an approved email on 2019-8-3.
>
> 19fabf2eafcb02b519d39a1b0a9dad5b9209db64
>
> * Constructor creates multiple cell nodes, but the
> Fixed a wasteful problem of creating all cells and deleting
On Wed, 26 Aug 2020 15:40:57 GMT, Jose Pereda wrote:
> So I'd like to propose this third alternative, which would affect only
> VirtualFlow and the controls that use it, but
> wouldn't have any impact in the rest of controls as the other two options (as
> ExpressionHelper or Node listeners
> wo
On Wed, 26 Aug 2020 17:44:20 GMT, yosbits
wrote:
>> So far, there are two alternative fixes for the bad performance issue while
>> scrolling TableView/TreeTableViews:
>>
>> - this one #108, that tries to improve the performance of excessive number
>> of `removeListener` calls
>> - the WIP #18
On Wed, 26 Aug 2020 15:40:57 GMT, Jose Pereda wrote:
>> I have attached a code sample. If you use OpenJFX 16-ea+1 and run visual VM
>> and look at the hotspots in the JavaFX
>> thread, you can see that about 45% of the time in the JavaFX thread is spent
>> in removeListener calls.
>> Note: In C
On Wed, 26 Aug 2020 14:08:37 GMT, dannygonzalez
wrote:
>> @hjohn, agreed regards the issues of adding a listener to each node.
>>
>> Would it be worth doing the additional work of updating PopupWindow and
>> ProgressIndicatorSkin to add their own
>> listeners to make this a pull request that c
On Fri, 17 Apr 2020 10:46:51 GMT, dannygonzalez
wrote:
>> The problem is that there are usually many nodes, but only very few scenes
>> and windows, so registering a listener for
>> each node on a scene or window is pretty bad design (also consider the
>> amount of notifications that a scene c
On Fri, 17 Apr 2020 10:22:15 GMT, John Hendrikx wrote:
>> @hjohn Thanks for looking into this. It looks like your changes do improve
>> the issue with the JavaFX thread being
>> swamped with listener de-registrations. Looking at the JavaFX thread in
>> VisualVM, the removeListener call has drop
On Fri, 17 Apr 2020 08:48:35 GMT, dannygonzalez
wrote:
>> @dannygonzalez I added a proof of concept here if you want to play with it:
>> https://github.com/openjdk/jfx/pull/185
>
> @hjohn Thanks for looking into this. It looks like your changes do improve
> the issue with the JavaFX thread bei
On Fri, 17 Apr 2020 08:07:08 GMT, John Hendrikx wrote:
>> @hjohn
>>
>>> 2. There is a significant increase in memory use. Where before each
>>> listener occupied an entry in an array, now each
>>> listener is wrapped by Map.Entry (the Integer instance used per entry can
>>> be disregarded). I
On Fri, 17 Apr 2020 07:22:20 GMT, dannygonzalez
wrote:
>> @hjon
>>
>>> 3. Even though the current version of this pull request takes care to
>>> notify duplicate listeners the correct amount of
>>> times, it does not notify them in the correct order with respect to other
>>> listeners. If one
On Fri, 17 Apr 2020 07:22:20 GMT, dannygonzalez
wrote:
>> @hjon
>>
>>> 3. Even though the current version of this pull request takes care to
>>> notify duplicate listeners the correct amount of
>>> times, it does not notify them in the correct order with respect to other
>>> listeners. If one
On Fri, 17 Apr 2020 07:08:01 GMT, dannygonzalez
wrote:
>> I've tested this pull request locally a few times, and the performance
>> improvement is quite significant. A test with
>> 20.000 nested stack panes resulted in these average times:
>> - Add all 51 ms
>> - Remove all 25 ms
>>
>> Versu
On Thu, 16 Apr 2020 16:15:20 GMT, John Hendrikx wrote:
>> @hjohn I have 12136 change listeners when debugging our application as you
>> suggested.
>>
>> Please note that I see the issue when the TableView is having items added to
>> it. If you just have a static TableView I
>> do not see the i
On Thu, 16 Apr 2020 09:45:20 GMT, dannygonzalez
wrote:
>> @dannygonzalez Could you perhaps debug your application and take a look at
>> how large the following array is: a random
>> node -> `scene` -> `value` -> `window` -> `readOnlyProperty` -> `helper` ->
>> `changeListeners`. I just teste
On Thu, 16 Apr 2020 08:46:26 GMT, John Hendrikx wrote:
>> If it is of any help, I have attached a VisualVM snapshot (v1.4.4) where the
>> ExpressionHelper.removeListener is using
>> 61% of the JavaFX thread whilst running our application.
>> [snapshot-1587024308245.nps.zip](https://github.com/op
On Thu, 16 Apr 2020 08:24:16 GMT, dannygonzalez
wrote:
>> The listeners added by `Node` are apparently internally required for
>> internal properties `TreeShowing` and
>> `TreeVisible`, and are used to take various decisions like whether to
>> play/pause animations. There is also a couple of
On Thu, 16 Apr 2020 07:34:40 GMT, John Hendrikx wrote:
>> Looking at the commit
>> https://github.com/openjdk/jfx/commit/e21606d3a1b73cd4b44383babc750a4b4721edfd
>> it seems that the long listener lists are actually part of the `Scene`'s
>> `Window` property and the `Window`'s `Showing`
>> p
On Thu, 16 Apr 2020 07:11:58 GMT, John Hendrikx wrote:
>> The patch proposed here does not share the case where the listener deletion
>> performance becomes a bottleneck.
>>
>> I think that it is necessary to reproduce it by testing first, but
>>
>> If you just focus on improving listener remo
On Thu, 16 Apr 2020 06:34:39 GMT, yosbits
wrote:
>> @hjohn
>> I haven't quantified exactly where all the listeners are coming from but the
>> Node class for example has various
>> listeners on it.
>> The following changeset (which added an extra listener on the Node class)
>> impacted TableVi
On Wed, 15 Apr 2020 07:11:27 GMT, dannygonzalez
wrote:
>> @dannygonzalez You mentioned "There are multiple change listeners on a Node
>> for example, so you can imagine with the
>> number of nodes in a TableView this is going to be a problem if the
>> unregistering is slow.".
>> Your changes i
On Tue, 14 Apr 2020 12:20:10 GMT, John Hendrikx wrote:
>> In a minimal test I wrote (not a microbenchmark that removes listeners), I
>> tried this PR code, but did not reproduce
>> the performance improvement. I have attached a test program in my PR(#125)
>
> @dannygonzalez You mentioned "There
On Tue, 10 Mar 2020 06:08:25 GMT, yosbits
wrote:
>>> technically true - but the implementation was linear with a fixed sequence
>>> since-the-beginning-of-java-desktop-time
>>> (and sometimes, for good ol' beans properties, even exposed as api to
>>> access the array of listeners). So technica
On Mon, 9 Mar 2020 17:00:06 GMT, Nir Lisker wrote:
>> I took the liberty of pointing out some formatting errors
>
> The title of your PR should match the title of the JBS issue. Moreover, #108
> already tackles this issue with a
> different approach and 2 PRs on the same issue can't be intetgrat
On Mon, 24 Feb 2020 07:39:43 GMT, yosbits
wrote:
> If there are many columns, the current TableView will stall scrolling.
> Resolving this performance issue requires column
> virtualization. Virtualization mode is enabled when the row height is fixed
> by the following method.
> `tableView.set
On Thu, 2 Apr 2020 17:38:00 GMT, Dalibor Topic wrote:
>> The title of your PR should match the title of the JBS issue. Moreover, #108
>> already tackles this issue with a
>> different approach and 2 PRs on the same issue can't be intetgrated. Since
>> I'm not familiar with this code, I can't
>>
On Thu, 27 Feb 2020 21:29:27 GMT, Martin Polakovic
wrote:
>> If there are many columns, the current TableView will stall scrolling.
>> Resolving this performance issue requires column
>> virtualization. Virtualization mode is enabled when the row height is fixed
>> by the following method.
>>
On Mon, 24 Feb 2020 07:39:43 GMT, yosbits
wrote:
> If there are many columns, the current TableView will stall scrolling.
> Resolving this performance issue requires column
> virtualization. Virtualization mode is enabled when the row height is fixed
> by the following method.
> `tableView.set
On Mon, 2 Mar 2020 11:47:03 GMT, Nir Lisker 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 be
On Mon, 2 Mar 2020 08:31:47 GMT, dannygonzalez
wrote:
>> In use cases where a large number of listeners are being discarded, it may
>> be better to first consider changing the design to receive event
>> notifications on nodes whose listener registrations are not frequently
>> discarded.
>
>>
On Thu, 27 Feb 2020 03:17:14 GMT, yosbits
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
On Wed, 26 Feb 2020 10:07:02 GMT, Jeanette Winzenburg
wrote:
>> 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
On Wed, 26 Feb 2020 09:49:04 GMT, Jeanette Winzenburg
wrote:
>> 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 >
On Tue, 25 Feb 2020 14:06:23 GMT, dannygonzalez
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
>> per
On Tue, 25 Feb 2020 13:55:50 GMT, Nir Lisker 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
On Tue, 25 Feb 2020 12:18:34 GMT, dannygonzalez
wrote:
>> I haven't done a detailed review yet, but I worry about the memory
>> consumption and performance of using a Map for the simple cases where there
>> is only a single (or very small number) of listeners. These methods are used
>> in man
On Tue, 25 Feb 2020 00:43:34 GMT, Kevin Rushforth wrote:
>> Sorry for the interruption, send a PR that corrects the same problem.
>
> I haven't done a detailed review yet, but I worry about the memory
> consumption and performance of using a Map for the simple cases where there
> is only a sin
On Tue, 25 Feb 2020 00:45:06 GMT, Kevin Rushforth wrote:
>> modules/javafx.base/src/main/java/com/sun/javafx/binding/ExpressionHelper.java
>> line 194:
>>
>>> 193: private Map
>>> invalidationListeners = new LinkedHashMap<>();
>>> 194: private Map, Integer>
>>> changeListeners
On Mon, 24 Feb 2020 23:45:03 GMT, Nir Lisker wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8185886
>>
>> Optimisation to ExpressionHelper.Generic class to use Sets rather than
>> Arrays to improve listener removal speed.
>>
>> This was due to the removal of listeners in TableView taking u
On Sat, 22 Feb 2020 08:42:52 GMT, yosbits
wrote:
>> @kevinrushforth just a note to say there are other ExpressionHelper classes
>> (i.e. MapExpressionHelper, SetExpressionHelper and ListExpressionHelper)
>> that also use arrays and suffer from the linear search issue when removing
>> listener
On Thu, 6 Feb 2020 16:13:33 GMT, dannygonzalez
wrote:
> https://bugs.openjdk.java.net/browse/JDK-8185886
>
> Optimisation to ExpressionHelper.Generic class to use Sets rather than Arrays
> to improve listener removal speed.
>
> This was due to the removal of listeners in TableView taking up t
On Mon, 24 Feb 2020 11:04:26 GMT, dannygonzalez
wrote:
>> Just wanted to keep a similar behaviour as before using the same calculation
>> based originally on the size of the listeners array list and now based on
>> the size of the map. So in theory the weak listeners should be trimmed at
>> t
On Mon, 24 Feb 2020 11:32:25 GMT, Nir Lisker wrote:
>> Nope, actually:
>> `listeners.keySet().removeIf(p::test) `
>>
>> is not the same as:
>> `listeners.entrySet().removeIf(e -> p.test(e.getKey()));`
>>
>> We need to test against the entrySet.key not the entrySet itself.
>
>> We need to test
On Mon, 24 Feb 2020 11:09:30 GMT, dannygonzalez
wrote:
>> Agreed, will change.
>
> Nope, actually:
> `listeners.keySet().removeIf(p::test) `
>
> is not the same as:
> `listeners.entrySet().removeIf(e -> p.test(e.getKey()));`
>
> We need to test against the entrySet.key not the entrySet itself
On Mon, 24 Feb 2020 08:11:40 GMT, dannygonzalez
wrote:
>> modules/javafx.base/src/main/java/com/sun/javafx/binding/ExpressionHelperBase.java
>> line 64:
>>
>>> 63:
>>> 64: listeners.entrySet().removeIf(e -> p.test(e.getKey()));
>>> 65: }
>>
>> This can be `listeners.keySet().remo
On Mon, 24 Feb 2020 10:16:55 GMT, dannygonzalez
wrote:
>>> So we need to keep the original behaviour from before where a count was
>>> checked on every addListener call and the weak references were purged from
>>> the array using the original calculation for this count.
>>
>> The GC'd weak li
On Mon, 24 Feb 2020 09:59:28 GMT, Nir Lisker wrote:
>> As far as I know a LinkedHashMap doesn't automatically remove weak listener
>> entries. The listener maps can be holding weak listeners as well as normal
>> listeners.
>> So we need to keep the original behaviour from before where a count w
On Mon, 24 Feb 2020 09:14:51 GMT, dannygonzalez
wrote:
>> I agree, I kept these in as I wasn't sure if there was a need to manually
>> force the garbage collection of weak listeners at the same rate as the
>> original implementation.
>> Removing this would make sense to me also.
>>
>> Updated
On Mon, 24 Feb 2020 08:14:02 GMT, dannygonzalez
wrote:
>> modules/javafx.base/src/main/java/com/sun/javafx/binding/ExpressionHelper.java
>> line 197:
>>
>>> 196: private int weakChangeListenerGcCount = 2;
>>> 197: private int weakInvalidationListenerGcCount = 2;
>>> 198:
>>
>
On Sun, 23 Feb 2020 01:05:25 GMT, Nir Lisker wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8185886
>>
>> Optimisation to ExpressionHelper.Generic class to use Sets rather than
>> Arrays to improve listener removal speed.
>>
>> This was due to the removal of listeners in TableView taking u
On Sun, 23 Feb 2020 02:54:55 GMT, Nir Lisker wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8185886
>>
>> Optimisation to ExpressionHelper.Generic class to use Sets rather than
>> Arrays to improve listener removal speed.
>>
>> This was due to the removal of listeners in TableView taking u
On Sun, 23 Feb 2020 03:09:28 GMT, Nir Lisker wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8185886
>>
>> Optimisation to ExpressionHelper.Generic class to use Sets rather than
>> Arrays to improve listener removal speed.
>>
>> This was due to the removal of listeners in TableView taking u
On Sun, 23 Feb 2020 04:20:55 GMT, Nir Lisker wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8185886
>>
>> Optimisation to ExpressionHelper.Generic class to use Sets rather than
>> Arrays to improve listener removal speed.
>>
>> This was due to the removal of listeners in TableView taking u
On Thu, 6 Feb 2020 16:13:33 GMT, dannygonzalez
wrote:
> https://bugs.openjdk.java.net/browse/JDK-8185886
>
> Optimisation to ExpressionHelper.Generic class to use Sets rather than Arrays
> to improve listener removal speed.
>
> This was due to the removal of listeners in TableView taking up t
On Tue, 18 Feb 2020 09:02:36 GMT, dannygonzalez
wrote:
>> @dannygonzalez the reason for the `jcheck` failure is that you have commits
>> with two different email addresses in your branch. At this point, it's
>> probably best to squash the commits into a single commit with `git rebase -i
>> ma
On Wed, 12 Feb 2020 12:43:58 GMT, dannygonzalez
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 begin
On Wed, 12 Feb 2020 12:22:15 GMT, Jeanette Winzenburg
wrote:
>>> hmm ... wouldn't the change violate spec of adding listeners:
>>>
>>> > If the same listener is added more than once, then it will be notified
>>> > more than once.
>>
>> True, I hadn't read that spec in ObservableValueBase.
>>
On Wed, 12 Feb 2020 13:06:00 GMT, Jeanette Winzenburg
wrote:
>> 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 b
On Thu, 6 Feb 2020 16:22:28 GMT, dannygonzalez
wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8185886
>>
>> Optimisation to ExpressionHelper.Generic class to use Sets rather than
>> Arrays to improve listener removal speed.
>>
>> This was due to the removal of listeners in TableView takin
On Mon, 17 Feb 2020 13:51:49 GMT, Kevin Rushforth wrote:
>>>
>>> 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 sequen
On Wed, 12 Feb 2020 11:29:24 GMT, Jeanette Winzenburg
wrote:
>> /signed
>>
>> I have signed the Oracle Contributor Agreement today. Have not received back
>> any confirmation yet though.
>
> hmm ... wouldn't the change violate spec of adding listeners:
>
>> If the same listener is added more
On Wed, 12 Feb 2020 12:01:03 GMT, dannygonzalez
wrote:
>> hmm ... wouldn't the change violate spec of adding listeners:
>>
>>> If the same listener is added more than once, then it will be notified more
>>> than once.
>
>> hmm ... wouldn't the change violate spec of adding listeners:
>>
>> >
On Thu, 6 Feb 2020 16:13:33 GMT, dannygonzalez
wrote:
> https://bugs.openjdk.java.net/browse/JDK-8185886
>
> Optimisation to ExpressionHelper.Generic class to use Sets rather than Arrays
> to improve listener removal speed.
>
> This was due to the removal of listeners in TableView taking up t
72 matches
Mail list logo
