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
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
> 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
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
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
> 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
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
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
>
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
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
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
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
On Fri, 3 Apr 2020 03:55:37 GMT, yosbits
wrote:
>> Hi,
>> I couldn't find you listed at
>> https://www.oracle.com/technetwork/community/oca-486395.html . Please send
>> me an e-mail
>> at dalibor.to...@oracle.com with information about your OCA, so that I can
>> look it up.
>
> My name is
> 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
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
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
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
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
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
>>
>>
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
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
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.
>>
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`
>>
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
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
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
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
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 fix because
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
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.
>
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.
>
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
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
>>
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
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
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>
>>>
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
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
>>
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
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
>>
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
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
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
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
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.
>>
>>
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
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
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
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
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
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
>>
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
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
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
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
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
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 to 50% of CPU
time on the JavaFX Application thread when
74 matches
Mail list logo
