Re: RFR: 8197991: Selecting many items in a TableView is very slow [v6]
On Fri, 7 Jan 2022 08:04:53 GMT, Abhinay Agarwal wrote: >> This work improves the performance of `MultipleSelectionModel` over large >> data sets by caching some values and avoiding unnecessary calls to >> `SelectedIndicesList#size`. It further improves the performance by reducing >> the number of iterations required to find the index of an element in the >> BitSet. >> >> The work is based on [an abandoned >> patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs >> >> There are currently 2 manual tests for this fix. > > Abhinay Agarwal has updated the pull request incrementally with two > additional commits since the last revision: > > - Format code and reduce test size > - Add size assertion to test The code changes look good. I tested on Mac. This PR drastically improves the selection time for the provided manual test. - Marked as reviewed by aghaisas (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v6]
On Fri, 7 Jan 2022 08:04:53 GMT, Abhinay Agarwal wrote: >> This work improves the performance of `MultipleSelectionModel` over large >> data sets by caching some values and avoiding unnecessary calls to >> `SelectedIndicesList#size`. It further improves the performance by reducing >> the number of iterations required to find the index of an element in the >> BitSet. >> >> The work is based on [an abandoned >> patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs >> >> There are currently 2 manual tests for this fix. > > Abhinay Agarwal has updated the pull request incrementally with two > additional commits since the last revision: > > - Format code and reduce test size > - Add size assertion to test Looks good. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v6]
> This work improves the performance of `MultipleSelectionModel` over large > data sets by caching some values and avoiding unnecessary calls to > `SelectedIndicesList#size`. It further improves the performance by reducing > the number of iterations required to find the index of an element in the > BitSet. > > The work is based on [an abandoned > patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs > > There are currently 2 manual tests for this fix. Abhinay Agarwal has updated the pull request incrementally with two additional commits since the last revision: - Format code and reduce test size - Add size assertion to test - Changes: - all: https://git.openjdk.java.net/jfx/pull/673/files - new: https://git.openjdk.java.net/jfx/pull/673/files/741aa92f..30fb31c4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=673=05 - incr: https://webrevs.openjdk.java.net/?repo=jfx=673=04-05 Stats: 16 lines in 3 files changed: 0 ins; 0 del; 16 mod Patch: https://git.openjdk.java.net/jfx/pull/673.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/673/head:pull/673 PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v5]
On Thu, 6 Jan 2022 21:01:11 GMT, Kevin Rushforth wrote: >> Abhinay Agarwal has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Remove commented out method. Document constructors. >> - Add tests > > modules/javafx.controls/src/test/java/test/javafx/scene/control/MultipleSelectionModelImplTest.java > line 1436: > >> 1434: model.clearSelection(); >> 1435: >> 1436: assertTrue(model.getSelectedIndices().isEmpty()); > > I recommend to also check the size. Isn't checking the size redundant in this case? `model.getSelectedIndices().isEmpty()` indicates that `model.getSelectedIndices().size()` is `0`. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v5]
On Thu, 6 Jan 2022 08:17:53 GMT, Abhinay Agarwal wrote:
>> This work improves the performance of `MultipleSelectionModel` over large
>> data sets by caching some values and avoiding unnecessary calls to
>> `SelectedIndicesList#size`. It further improves the performance by reducing
>> the number of iterations required to find the index of an element in the
>> BitSet.
>>
>> The work is based on [an abandoned
>> patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs
>>
>> There are currently 2 manual tests for this fix.
>
> Abhinay Agarwal has updated the pull request incrementally with two
> additional commits since the last revision:
>
> - Remove commented out method. Document constructors.
> - Add tests
The fix looks good. I left a few comments on the tests.
modules/javafx.controls/src/test/java/test/javafx/scene/control/MultipleSelectionModelImplTest.java
line 1427:
> 1425: assertTrue(model.isSelected(2));
> 1426: assertTrue(model.isSelected(5));
> 1427: assertFalse(model.isSelected(0));
I recommend to also check the size.
modules/javafx.controls/src/test/java/test/javafx/scene/control/MultipleSelectionModelImplTest.java
line 1436:
> 1434: model.clearSelection();
> 1435:
> 1436: assertTrue(model.getSelectedIndices().isEmpty());
I recommend to also check the size.
tests/manual/controls/SelectTableViewTest.java line 40:
> 38: public class SelectTableViewTest extends Application {
> 39:
> 40: final int ROW_COUNT = 700_000;
This count is too large, even with the fix. I recommend changing it to
something smaller (no more than `70_000`, which matches what you did for
`SelectListViewTest`).
tests/manual/controls/SelectTableViewTest.java line 101:
> 99: long t = System.currentTimeMillis();
> 100: tableView.getSelectionModel().selectAll();
> 101: System.out.println("time:"+ (System.currentTimeMillis() - t));
Minor: space before the `+` here and a few places below (also in the other test
as noted).
-
PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Fri, 17 Dec 2021 20:38:56 GMT, Kevin Rushforth wrote:
>> Abhinay Agarwal has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Update ROW_COUNT to 700_000
>
> tests/manual/controls/SelectListViewTest.java line 66:
>
>> 64: long t = System.currentTimeMillis();
>> 65: listView.getSelectionModel().clearSelection();
>> 66: System.out.println("time:"+ (System.currentTimeMillis() - t));
>
> MInor: space before the `+` here and a few places below.
This is still pending.
-
PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Wed, 22 Dec 2021 13:32:06 GMT, Kevin Rushforth wrote: >> A cache that's manually invalidated and then validated when needed is a form >> of lazy evaluation. The main point, regardless of how you name it, is to >> ensure that every call that modifies the underlying BitSet invalidates the >> size. If it does, and it can be proven to do so, then that's sufficient. > > I checked, and I think that `set(int index, int... indices)` is fine, since > it doesn't modify `bitset` directly, and all of the methods it calls that do > modify it, correctly invalidate `size`. > > One more thing I spotted that needs to be checked, is the > `SelectedIndicesList(BitSet)` constructor. It saves a reference to the source > `BitSet` without making a copy. If a caller ever modified the source > `BitSet`, it would change the underlying `BitSet` seen by the > `SelectedIndicesList` class, and the size would therefore be wrong. Even if > the current code doesn't do this, it's somewhat fragile. I recommend adding a > comment to the constructor, and to the one place it's called, noting that the > source `BitSet` must not be modified after passing it to the constructor. > > Finally, there is a commented out `clearAndSelect` method that would break > the new contract of invalidating size whenever the bitmap is modified, if > that method were ever uncommented. Either it should be deleted as part of > this PR or it should be modified with a (commented out, of course) `size = > -1` in the right place(s). I have added some tests which calls public methods in `MultipleSelectionModelBase` and in turn calls methods from `SelectedIndicesList`. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v5]
> This work improves the performance of `MultipleSelectionModel` over large > data sets by caching some values and avoiding unnecessary calls to > `SelectedIndicesList#size`. It further improves the performance by reducing > the number of iterations required to find the index of an element in the > BitSet. > > The work is based on [an abandoned > patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs > > There are currently 2 manual tests for this fix. Abhinay Agarwal has updated the pull request incrementally with two additional commits since the last revision: - Remove commented out method. Document constructors. - Add tests - Changes: - all: https://git.openjdk.java.net/jfx/pull/673/files - new: https://git.openjdk.java.net/jfx/pull/673/files/2d321087..741aa92f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=673=04 - incr: https://webrevs.openjdk.java.net/?repo=jfx=673=03-04 Stats: 98 lines in 2 files changed: 58 ins; 40 del; 0 mod Patch: https://git.openjdk.java.net/jfx/pull/673.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/673/head:pull/673 PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v4]
On Tue, 21 Dec 2021 12:21:57 GMT, Abhinay Agarwal wrote: >> This work improves the performance of `MultipleSelectionModel` over large >> data sets by caching some values and avoiding unnecessary calls to >> `SelectedIndicesList#size`. It further improves the performance by reducing >> the number of iterations required to find the index of an element in the >> BitSet. >> >> The work is based on [an abandoned >> patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs >> >> There are currently 2 manual tests for this fix. > > Abhinay Agarwal has updated the pull request incrementally with two > additional commits since the last revision: > > - Add space around operator and string > - Add license header and add space around operators I think this is very close to being ready to go in. I left one more inline comment about the invalidation of `size`. There is also a pending minor code formatting comment. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Tue, 21 Dec 2021 17:56:22 GMT, Kevin Rushforth wrote: >> This is a cache of the cardinality method of BitSet, not Lazy evaluation. It >> sets invalidity (-1) in the method call that changes the bit. This could be >> a subclass of BitSet to make the code more readable. This patch does not >> subclass BitSet because there is no case where it is used in the same way. >> >> If you subclass it, you can probably reduce the number of calls to >> cardinality to zero if you calculate it wisely instead of simply setting >> invalid. The patch does not take this challenge for readability reasons. > > A cache that's manually invalidated and then validated when needed is a form > of lazy evaluation. The main point, regardless of how you name it, is to > ensure that every call that modifies the underlying BitSet invalidates the > size. If it does, and it can be proven to do so, then that's sufficient. I checked, and I think that `set(int index, int... indices)` is fine, since it doesn't modify `bitset` directly, and all of the methods it calls that do modify it, correctly invalidate `size`. One more thing I spotted that needs to be checked, is the `SelectedIndicesList(BitSet)` constructor. It saves a reference to the source `BitSet` without making a copy. If a caller ever modified the source `BitSet`, it would change the underlying `BitSet` seen by the `SelectedIndicesList` class, and the size would therefore be wrong. Even if the current code doesn't do this, it's somewhat fragile. I recommend adding a comment to the constructor, and to the one place it's called, noting that the source `BitSet` must not be modified after passing it to the constructor. Finally, there is a commented out `clearAndSelect` method that would break the new contract of invalidating size whenever the bitmap is modified, if that method were ever uncommented. Either it should be deleted as part of this PR or it should be modified with a (commented out, of course) `size = -1` in the right place(s). - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Tue, 21 Dec 2021 17:08:26 GMT, yosbits wrote: >> What about the first part of my question? Have you looked at the set logic >> to ensure that `size` is being invalidated in all places it should be. The >> `set(int index, int... indices)` method does not directly invalidate `size`. >> If that method unconditionally set `size = -1`, it would be easier to prove >> that there are no missed cases. > > This is a cache of the cardinality method of BitSet, not Lazy evaluation. It > sets invalidity (-1) in the method call that changes the bit. This could be a > subclass of BitSet to make the code more readable. This patch does not > subclass BitSet because there is no case where it is used in the same way. > > If you subclass it, you can probably reduce the number of calls to > cardinality to zero if you calculate it wisely instead of simply setting > invalid. The patch does not take this challenge for readability reasons. A cache that's manually invalidated and then validated when needed is a form of lazy evaluation. The main point, regardless of how you name it, is to ensure that every call that modifies the underlying BitSet invalidates the size. If it does, and it can be proven to do so, then that's sufficient. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Tue, 21 Dec 2021 15:35:32 GMT, Kevin Rushforth wrote: >> Good point. All the test cases that I could think of were already present in >> `MultipleSelectionModelImplTest`. Nevertheless, test cases for different >> `set()` methods can definitely be added. I will work on it. > > What about the first part of my question? Have you looked at the set logic to > ensure that `size` is being invalidated in all places it should be. The > `set(int index, int... indices)` method does not directly invalidate `size`. > If that method unconditionally set `size = -1`, it would be easier to prove > that there are no missed cases. This is a cache of the cardinality method of BitSet, not Lazy evaluation. It sets invalidity (-1) in the method call that changes the bit. This could be a subclass of BitSet to make the code more readable. This patch does not subclass BitSet because there is no case where it is used in the same way. If it were to be subclassed, it would be possible to reduce the number of cardinality calls if it were calculated wisely instead of simply being disabled. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Tue, 21 Dec 2021 12:13:55 GMT, Abhinay Agarwal wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java
>> line 877:
>>
>>> 875: if (size >= 0) {
>>> 876: return size;
>>> 877: }
>>
>> Using lazy evaluation means that you need to be extra careful that the size
>> is invalidated in all the right places. One method that needs to be checked
>> is the `set(int index, int... indices)` method. How carefully have you
>> checked to make sure that nothing that could change the size fails to update
>> the `size` field?
>>
>> Related to this, are you satisfied that the current set of unit tests are
>> sufficient to catch any potential problems, and that you don't need to add
>> more tests?
>
> Good point. All the test cases that I could think of were already present in
> `MultipleSelectionModelImplTest`. Nevertheless, test cases for different
> `set()` methods can definitely be added. I will work on it.
What about the first part of my question? Have you looked at the set logic to
ensure that `size` is being invalidated in all places it should be. The
`set(int index, int... indices)` method does not directly invalidate `size`. If
that method unconditionally set `size = -1`, it would be easier to prove that
there are no missed cases.
-
PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Fri, 17 Dec 2021 17:46:54 GMT, Kevin Rushforth wrote:
>> Abhinay Agarwal has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Update ROW_COUNT to 700_000
>
> modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java
> line 877:
>
>> 875: if (size >= 0) {
>> 876: return size;
>> 877: }
>
> Using lazy evaluation means that you need to be extra careful that the size
> is invalidated in all the right places. One method that needs to be checked
> is the `set(int index, int... indices)` method. How carefully have you
> checked to make sure that nothing that could change the size fails to update
> the `size` field?
>
> Related to this, are you satisfied that the current set of unit tests are
> sufficient to catch any potential problems, and that you don't need to add
> more tests?
Good point. All the test cases that I could think of were already present in
`MultipleSelectionModelImplTest`. Nevertheless, test cases for different
`set()` methods can definitely be added. I will work on it.
-
PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v4]
> This work improves the performance of `MultipleSelectionModel` over large > data sets by caching some values and avoiding unnecessary calls to > `SelectedIndicesList#size`. It further improves the performance by reducing > the number of iterations required to find the index of an element in the > BitSet. > > The work is based on [an abandoned > patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs > > There are currently 2 manual tests for this fix. Abhinay Agarwal has updated the pull request incrementally with two additional commits since the last revision: - Add space around operator and string - Add license header and add space around operators - Changes: - all: https://git.openjdk.java.net/jfx/pull/673/files - new: https://git.openjdk.java.net/jfx/pull/673/files/8196b348..2d321087 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=673=03 - incr: https://webrevs.openjdk.java.net/?repo=jfx=673=02-03 Stats: 68 lines in 2 files changed: 50 ins; 0 del; 18 mod Patch: https://git.openjdk.java.net/jfx/pull/673.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/673/head:pull/673 PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Fri, 17 Dec 2021 19:18:23 GMT, Kevin Rushforth wrote:
>> Abhinay Agarwal has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Update ROW_COUNT to 700_000
>
> modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java
> line 880:
>
>> 878: @Override public int indexOf(Object obj) {
>> 879: reset();
>> 880: return super.indexOf(obj);
>
> So `reset` doesn't need to still be called?
`reset()` was previously called since `super.indexof()` was making calls to
`get(int index)` and it needed to have `lastGetIndex` and `lastGetValue` set to
-1
-
PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Fri, 26 Nov 2021 19:49:37 GMT, Abhinay Agarwal wrote: >> This work improves the performance of `MultipleSelectionModel` over large >> data sets by caching some values and avoiding unnecessary calls to >> `SelectedIndicesList#size`. It further improves the performance by reducing >> the number of iterations required to find the index of an element in the >> BitSet. >> >> The work is based on [an abandoned >> patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs >> >> There are currently 2 manual tests for this fix. > > Abhinay Agarwal has updated the pull request incrementally with one > additional commit since the last revision: > > Update ROW_COUNT to 700_000 Thanks for adding the manual test. I can confirm the performance gains. I ran the ListView test, clicked on one of the cells visible without scrolling, then pressed the "select to the end" operation. With the 70k cells defined by the test program that operation ran 13,715 times faster on my machine with this fix. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Wed, 24 Nov 2021 14:39:57 GMT, yosbits wrote: >> But why? The initialization block of a `for` statement is exactly where >> you'd put loop-scoped variables. > > This change significantly improves performance because the BitSet's size () > method is an O (N) implementation. You may be wondering because it is O (1) > in many other collections. This change looks fine to me. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Fri, 26 Nov 2021 19:49:37 GMT, Abhinay Agarwal wrote:
>> This work improves the performance of `MultipleSelectionModel` over large
>> data sets by caching some values and avoiding unnecessary calls to
>> `SelectedIndicesList#size`. It further improves the performance by reducing
>> the number of iterations required to find the index of an element in the
>> BitSet.
>>
>> The work is based on [an abandoned
>> patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs
>>
>> There are currently 2 manual tests for this fix.
>
> Abhinay Agarwal has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Update ROW_COUNT to 700_000
The fix looks good to me. I did leave a couple questions about the
implementation.
The new manual tests need a copyright header, and I noted a few of the code
formatting issues.
modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java
line 877:
> 875: if (size >= 0) {
> 876: return size;
> 877: }
Using lazy evaluation means that you need to be extra careful that the size is
invalidated in all the right places. One method that needs to be checked is the
`set(int index, int... indices)` method. How carefully have you checked to make
sure that nothing that could change the size fails to update the `size` field?
Related to this, are you satisfied that the current set of unit tests are
sufficient to catch any potential problems, and that you don't need to add more
tests?
modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java
line 880:
> 878: @Override public int indexOf(Object obj) {
> 879: reset();
> 880: return super.indexOf(obj);
So `reset` doesn't need to still be called?
tests/manual/controls/SelectListViewTest.java line 1:
> 1: import javafx.application.Application;
This needs a copyright header.
tests/manual/controls/SelectListViewTest.java line 24:
> 22:
> 23: ObservableList items = listView.getItems();
> 24: for(int i=0; i 45: stage.setScene(new Scene(root, 600, 600));
> 46:
> 47: selectAll.setOnAction((e)->selectAll(listView));
Minor: add space before and after the `->` operator.
tests/manual/controls/SelectListViewTest.java line 66:
> 64: long t = System.currentTimeMillis();
> 65: listView.getSelectionModel().clearSelection();
> 66: System.out.println("time:"+ (System.currentTimeMillis() - t));
MInor: space before the `+` here and a few places below.
tests/manual/controls/SelectTableViewTest.java line 1:
> 1: import javafx.application.Application;
This needs a copyright header.
tests/manual/controls/SelectTableViewTest.java line 28:
> 26:
> 27: final ObservableList> columns =
> tableView.getColumns();
> 28: for(int i=0; i 35:
> 36: ObservableList items = tableView.getItems();
> 37: for(int i=0; i 37: for(int i=0; i 38: String[] rec = new String[COL_COUNT];
> 39: for(int j=0; jhttps://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Tue, 23 Nov 2021 17:40:29 GMT, Kevin Rushforth wrote: >> This work improves the performance of `MultipleSelectionModel` over large >> data sets by caching some values and avoiding unnecessary calls to >> `SelectedIndicesList#size`. It further improves the performance by reducing >> the number of iterations required to find the index of an element in the >> BitSet. >> >> The work is based on [an abandoned >> patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs >> >> There are currently 2 manual tests for this fix. > > I see that the `/contributor` command didn't work with the original > contributor's GitHub username. You can instead take the contributor > information from the HEAD commit of the pull request: `Naohiro Yoshimoto > yosb...@gmail.com`. @kevinrushforth Is there something I can do to move this PR forward? - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Fri, 26 Nov 2021 19:44:14 GMT, Abhinay Agarwal wrote:
>> tests/manual/controls/SelectTableViewTest.java line 19:
>>
>>> 17: // final int ROW_COUNT = 80_000;
>>> 18: // final int ROW_COUNT = 50_000;
>>> 19: // final int ROW_COUNT = 8_000;
>>
>> The number is meaningful because it is the number of data I used to show the
>> improvement effect in the original PR.
>>
>> Reference value of the number of data that can be handled within 3 seconds
>> of processing time (before-> after)
>>
>> TableView
>>
>> selectAll: 8_000-> 700_000
>> selectRange: 7_000-> 50_000
>>
>>
>>
>>
>>
>> ``` Java
>> public class SelectTableViewTest extends Application {
>>
>> final int ROW_COUNT = 700_000;
>> // final int ROW_COUNT = 80_000;
>> // final int ROW_COUNT = 50_000;
>> // final int ROW_COUNT = 8_000;
>> final int COL_COUNT = 3;
>
> I reduced ROW_COUNT from 700_000 to 70_000 as the tests were taking a few
> seconds to run on my machine. I have reverted these now. Nevertheless, time
> taken to run a test have a number of variables. Depending on the machine the
> tests are run, it may not necessarily always take 3 seconds :)
The processing time changes for each operation.
You probably adapted to the slow operation (selectRange).
You can comment it out, but 700_000 is for selectAll. Although it depends on
the device, the environment I have confirmed is his MacBook Pro in 2016.
-
PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Fri, 26 Nov 2021 13:10:08 GMT, yosbits wrote:
>> Abhinay Agarwal has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Update ROW_COUNT to 700_000
>
> tests/manual/controls/SelectTableViewTest.java line 19:
>
>> 17: // final int ROW_COUNT = 80_000;
>> 18: // final int ROW_COUNT = 50_000;
>> 19: // final int ROW_COUNT = 8_000;
>
> The number is meaningful because it is the number of data I used to show the
> improvement effect in the original PR.
>
> Reference value of the number of data that can be handled within 3 seconds of
> processing time (before-> after)
>
> TableView
>
> selectAll: 8_000-> 700_000
> selectRange: 7_000-> 50_000
>
>
>
>
>
> ``` Java
> public class SelectTableViewTest extends Application {
>
> final int ROW_COUNT = 700_000;
> //final int ROW_COUNT = 80_000;
> //final int ROW_COUNT = 50_000;
> //final int ROW_COUNT = 8_000;
> final int COL_COUNT = 3;
I reduced ROW_COUNT from 700_000 to 70_000 as the tests were taking a few
seconds to run on my machine. I have reverted these now. Nevertheless, time
taken to run a test have a number of variables. Depending on the machine the
tests are run, it may not necessarily always take 3 seconds :)
-
PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
> This work improves the performance of `MultipleSelectionModel` over large > data sets by caching some values and avoiding unnecessary calls to > `SelectedIndicesList#size`. It further improves the performance by reducing > the number of iterations required to find the index of an element in the > BitSet. > > The work is based on [an abandoned > patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs > > There are currently 2 manual tests for this fix. Abhinay Agarwal has updated the pull request incrementally with one additional commit since the last revision: Update ROW_COUNT to 700_000 - Changes: - all: https://git.openjdk.java.net/jfx/pull/673/files - new: https://git.openjdk.java.net/jfx/pull/673/files/3158acd7..8196b348 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=673=02 - incr: https://webrevs.openjdk.java.net/?repo=jfx=673=01-02 Stats: 3 lines in 1 file changed: 1 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/673.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/673/head:pull/673 PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v2]
On Fri, 26 Nov 2021 12:54:59 GMT, yosbits wrote:
>> Abhinay Agarwal has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove duplicate entry for test data
>
> tests/manual/controls/SelectListViewTest.java line 18:
>
>> 16: // final int ROW_COUNT = 10_000_000;
>> 17: // final int ROW_COUNT = 7_000;
>> 18:
>
> There is a duplication of the number of test data.
>
> My test is below.
>
> ``` Java
> public class SelectListViewTest extends Application {
> final int ROW_COUNT = 70_000;
> //final int ROW_COUNT = 400_000;
> //final int ROW_COUNT = 10_000_000;
> //final int ROW_COUNT = 7_000;
Thanks. I have fixed this.
-
PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v2]
> This work improves the performance of `MultipleSelectionModel` over large > data sets by caching some values and avoiding unnecessary calls to > `SelectedIndicesList#size`. It further improves the performance by reducing > the number of iterations required to find the index of an element in the > BitSet. > > The work is based on [an abandoned > patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs > > There are currently 2 manual tests for this fix. Abhinay Agarwal has updated the pull request incrementally with one additional commit since the last revision: Remove duplicate entry for test data - Changes: - all: https://git.openjdk.java.net/jfx/pull/673/files - new: https://git.openjdk.java.net/jfx/pull/673/files/05f5df32..3158acd7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=673=01 - incr: https://webrevs.openjdk.java.net/?repo=jfx=673=00-01 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/673.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/673/head:pull/673 PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Wed, 17 Nov 2021 05:34:46 GMT, Abhinay Agarwal wrote:
> This work improves the performance of `MultipleSelectionModel` over large
> data sets by caching some values and avoiding unnecessary calls to
> `SelectedIndicesList#size`. It further improves the performance by reducing
> the number of iterations required to find the index of an element in the
> BitSet.
>
> The work is based on [an abandoned
> patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs
>
> There are currently 2 manual tests for this fix.
tests/manual/controls/SelectTableViewTest.java line 19:
> 17: // final int ROW_COUNT = 80_000;
> 18: // final int ROW_COUNT = 50_000;
> 19: // final int ROW_COUNT = 8_000;
The number is meaningful because it is the number of data I used to show the
improvement effect in the original PR.
Reference value of the number of data that can be handled within 3 seconds of
processing time (before-> after)
TableView
selectAll: 8_000-> 700_000
selectRange: 7_000-> 50_000
``` Java
public class SelectTableViewTest extends Application {
final int ROW_COUNT = 700_000;
// final int ROW_COUNT = 80_000;
// final int ROW_COUNT = 50_000;
// final int ROW_COUNT = 8_000;
final int COL_COUNT = 3;
-
PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Wed, 17 Nov 2021 05:34:46 GMT, Abhinay Agarwal wrote:
> This work improves the performance of `MultipleSelectionModel` over large
> data sets by caching some values and avoiding unnecessary calls to
> `SelectedIndicesList#size`. It further improves the performance by reducing
> the number of iterations required to find the index of an element in the
> BitSet.
>
> The work is based on [an abandoned
> patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs
>
> There are currently 2 manual tests for this fix.
tests/manual/controls/SelectListViewTest.java line 18:
> 16: // final int ROW_COUNT = 10_000_000;
> 17: // final int ROW_COUNT = 7_000;
> 18:
There is a duplication of the number of test data.
My test is below.
``` Java
public class SelectListViewTest extends Application {
final int ROW_COUNT = 70_000;
// final int ROW_COUNT = 400_000;
// final int ROW_COUNT = 10_000_000;
// final int ROW_COUNT = 7_000;
-
PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Mon, 22 Nov 2021 16:25:22 GMT, Michael Strauß wrote: >> I could move `int max = size();` outside the loop > > But why? The initialization block of a `for` statement is exactly where you'd > put loop-scoped variables. This change significantly improves performance because the BitSet's size () method is an O (N) implementation. You may be wondering because it is O (1) in many other collections. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Wed, 17 Nov 2021 05:34:46 GMT, Abhinay Agarwal wrote: > This work improves the performance of `MultipleSelectionModel` over large > data sets by caching some values and avoiding unnecessary calls to > `SelectedIndicesList#size`. It further improves the performance by reducing > the number of iterations required to find the index of an element in the > BitSet. > > The work is based on [an abandoned > patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs > > There are currently 2 manual tests for this fix. I see that the `/contributor` command didn't work with the original contributor's GitHub username. You can instead take the contributor information from the HEAD commit of the pull request: `Naohiro Yoshimoto yosb...@gmail.com`. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Mon, 22 Nov 2021 15:55:03 GMT, Abhinay Agarwal wrote: >> the for-loop is certainly faster and would allocate less memory - i find the >> `for(int i = 0, max = size())`-style a bit odd > > I could move `int max = size();` outside the loop But why? The initialization block of a `for` statement is exactly where you'd put loop-scoped variables. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Thu, 18 Nov 2021 08:53:07 GMT, Marius Hanl wrote: >> This work improves the performance of `MultipleSelectionModel` over large >> data sets by caching some values and avoiding unnecessary calls to >> `SelectedIndicesList#size`. It further improves the performance by reducing >> the number of iterations required to find the index of an element in the >> BitSet. >> >> The work is based on [an abandoned >> patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs >> >> There are currently 2 manual tests for this fix. > > Just wondering, isn't it also possible to write some unit tests for the > MultipleSelectionModel(Base) ? @Maran23 I don't know what would be an apt unit test as the PR changes implementation detail. If you have suggestions, please let me know. There are 276 unit tests in `test.javafx.scene.control.MultipleSelectionModelImplTest` and all of them still pass with the changes made to this PR. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Thu, 18 Nov 2021 00:54:30 GMT, Nir Lisker wrote: >> This work improves the performance of `MultipleSelectionModel` over large >> data sets by caching some values and avoiding unnecessary calls to >> `SelectedIndicesList#size`. It further improves the performance by reducing >> the number of iterations required to find the index of an element in the >> BitSet. >> >> The work is based on [an abandoned >> patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs >> >> There are currently 2 manual tests for this fix. > > modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/ReadOnlyUnbackedObservableList.java > line 193: > >> 191: arr[i] = get(i); >> 192: } >> 193: return arr; > > Have you tried `return stream().toArray();`? I haven't measured how `stream().toArray()` compare to a for..loop, but I inherently try to avoid streams in such scenarios. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Thu, 18 Nov 2021 09:06:08 GMT, Tom Schindl wrote: >> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/ReadOnlyUnbackedObservableList.java >> line 119: >> >>> 117: Object obj = get(i); >>> 118: if (o.equals(obj)) return i; >>> 119: } >> >> An alternative is >> >> return IntStream.range(0, size()) >> .filter(i -> o.equals(get(i))) >> .findFirst() >> .orElse(-1); >> >> I don't know if it helps. > > the for-loop is certainly faster and would allocate less memory - i find the > `for(int i = 0, max = size())`-style a bit odd I could move `int max = size();` outside the loop - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Thu, 18 Nov 2021 00:55:18 GMT, Nir Lisker wrote: >> This work improves the performance of `MultipleSelectionModel` over large >> data sets by caching some values and avoiding unnecessary calls to >> `SelectedIndicesList#size`. It further improves the performance by reducing >> the number of iterations required to find the index of an element in the >> BitSet. >> >> The work is based on [an abandoned >> patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs >> >> There are currently 2 manual tests for this fix. > > modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/ReadOnlyUnbackedObservableList.java > line 119: > >> 117: Object obj = get(i); >> 118: if (o.equals(obj)) return i; >> 119: } > > An alternative is > > return IntStream.range(0, size()) > .filter(i -> o.equals(get(i))) > .findFirst() > .orElse(-1); > > I don't know if it helps. the for-loop is certainly faster and would allocate less memory - i find the `for(int i = 0, max = size())`-style a bit odd - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Wed, 17 Nov 2021 05:34:46 GMT, Abhinay Agarwal wrote: > This work improves the performance of `MultipleSelectionModel` over large > data sets by caching some values and avoiding unnecessary calls to > `SelectedIndicesList#size`. It further improves the performance by reducing > the number of iterations required to find the index of an element in the > BitSet. > > The work is based on [an abandoned > patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs > > There are currently 2 manual tests for this fix. Just wondering, isn't it also possible to write some unit tests for the MultipleSelectionModel(Base) ? - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Wed, 17 Nov 2021 05:34:46 GMT, Abhinay Agarwal wrote: > This work improves the performance of `MultipleSelectionModel` over large > data sets by caching some values and avoiding unnecessary calls to > `SelectedIndicesList#size`. It further improves the performance by reducing > the number of iterations required to find the index of an element in the > BitSet. > > The work is based on [an abandoned > patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs > > There are currently 2 manual tests for this fix. modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/ReadOnlyUnbackedObservableList.java line 119: > 117: Object obj = get(i); > 118: if (o.equals(obj)) return i; > 119: } An alternative is return IntStream.range(0, size()) .filter(i -> o.equals(get(i))) .findFirst() .orElse(-1); I don't know if it helps. modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/ReadOnlyUnbackedObservableList.java line 193: > 191: arr[i] = get(i); > 192: } > 193: return arr; Have you tried `return stream().toArray();`? - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Wed, 17 Nov 2021 05:34:46 GMT, Abhinay Agarwal wrote: > This work improves the performance of `MultipleSelectionModel` over large > data sets by caching some values and avoiding unnecessary calls to > `SelectedIndicesList#size`. It further improves the performance by reducing > the number of iterations required to find the index of an element in the > BitSet. > > The work is based on [an abandoned > patch](https://github.com/openjdk/jfx/pull/127) submitted by @yososs > > There are currently 2 manual tests for this fix. I closed #127 on which this PR is based. @abhinayagarwal Please use the `/contributor` command to add @yososs as a contributor. - PR: https://git.openjdk.java.net/jfx/pull/673
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v4]
On Mon, 12 Apr 2021 08:34:25 GMT, yosbits wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8197991
>>
>> The performance of the selectAll and selectRange methods of the
>> MultiSelectionModel class has been greatly improved.
>>
>> This greatly improves the response when selecting multiple items in ListView
>> and TableView.
>>
>> However, in TreeView / TreeTableView, the improvement effect is hidden
>> mainly due to the design problem of the cache of TreeUtil.getTreeItem ().
>>
>> Reference value of the number of data that can be handled within 3 seconds
>> of processing time (before-> after)
>>
>> ListView
>> * selectAll: 400_000-> 10_000_000
>> * selectRange: 7_000-> 70_000
>>
>> TableView
>> * selectAll: 8_000-> 700_000
>> * selectRange: 7_000-> 50_000
>>
>>
>> You can see performance improvements in the following applications:
>>
>>
>> ``` SelectListViewTest.java
>> import javafx.application.Application;
>> import javafx.collections.ObservableList;
>> import javafx.scene.Scene;
>> import javafx.scene.control.Button;
>> import javafx.scene.control.ListView;
>> import javafx.scene.control.SelectionMode;
>> import javafx.scene.layout.BorderPane;
>> import javafx.scene.layout.VBox;
>> import javafx.stage.Stage;
>>
>> public class SelectListViewTest extends Application {
>> final int ROW_COUNT = 70_000;
>> // final int ROW_COUNT = 400_000;
>> // final int ROW_COUNT = 10_000_000;
>> // final int ROW_COUNT = 7_000;
>>
>> @Override
>> public void start(Stage stage) {
>> ListView listView = new ListView<>();
>> listView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
>>
>>
>> ObservableList items = listView.getItems();
>> for(int i=0; i> String rec = String.valueOf(i);
>> items.add(rec);
>> }
>> BorderPane root = new BorderPane(listView);
>> Button selectAll = new Button("selectAll");
>> Button clearSelection = new Button("clearSelection");
>> Button selectToStart = new Button("selectToStart");
>> Button selectToEnd = new Button("selectToEnd");
>> Button selectPrevious = new Button("selectPrevious");
>> Button selectNext= new Button("selectNext");
>>
>> selectAll.setFocusTraversable(true);
>> clearSelection.setFocusTraversable(true);
>> selectToStart.setFocusTraversable(true);
>> selectToEnd.setFocusTraversable(true);
>> selectPrevious.setFocusTraversable(true);
>> selectNext.setFocusTraversable(true);
>>
>> root.setRight(new VBox(6, selectAll, selectToStart, selectToEnd,
>> selectPrevious, selectNext, clearSelection));
>> stage.setScene(new Scene(root, 600, 600));
>>
>> selectAll.setOnAction((e)->selectAll(listView));
>> clearSelection.setOnAction((e)->clearSelection(listView));
>> selectToStart.setOnAction((e)->selectToStart(listView));
>> selectToEnd.setOnAction((e)->selectToLast(listView));
>> selectPrevious.setOnAction((e)->selectPrevious(listView));
>> selectNext.setOnAction((e)->selectNext(listView));
>>
>> stage.show();
>> }
>>
>> private void selectAll(ListView listView) {
>> long t = System.currentTimeMillis();
>> listView.getSelectionModel().selectAll();
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>> private void clearSelection(ListView listView) {
>> long t = System.currentTimeMillis();
>> listView.getSelectionModel().clearSelection();
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>> private void selectToStart(ListView listView) {
>> long t = System.currentTimeMillis();
>> listView.getSelectionModel().selectRange(0,
>> listView.getSelectionModel().getSelectedIndex());
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>> private void selectToLast(ListView listView) {
>> long t = System.currentTimeMillis();
>>
>> listView.getSelectionModel().selectRange(listView.getSelectionModel().getSelectedIndex(),
>> listView.getItems().size());
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>>
>> private void selectPrevious(ListView listView) {
>> long t = System.currentTimeMillis();
>> listView.getSelectionModel().selectPrevious();
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>>
>> private void selectNext(ListView listView) {
>> long t = System.currentTimeMillis();
>> listView.getSelectionModel().selectNext();
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>> public static void main(String[] args) {
>> Application.launch(args);
>> }
>> }
>>
>>
>> ```
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Wed, 26 Feb 2020 07:37:06 GMT, yosbits
wrote:
> https://bugs.openjdk.java.net/browse/JDK-8197991
>
> The performance of the selectAll and selectRange methods of the
> MultiSelectionModel class has been greatly improved.
>
> This greatly improves the response when selecting multiple items in ListView
> and TableView.
>
> However, in TreeView / TreeTableView, the improvement effect is hidden mainly
> due to the design problem of the cache of TreeUtil.getTreeItem ().
>
> Reference value of the number of data that can be handled within 3 seconds of
> processing time (before-> after)
>
> ListView
> * selectAll: 400_000-> 10_000_000
> * selectRange: 7_000-> 70_000
>
> TableView
> * selectAll: 8_000-> 700_000
> * selectRange: 7_000-> 50_000
>
>
> You can see performance improvements in the following applications:
>
>
> ``` SelectListViewTest.java
> import javafx.application.Application;
> import javafx.collections.ObservableList;
> import javafx.scene.Scene;
> import javafx.scene.control.Button;
> import javafx.scene.control.ListView;
> import javafx.scene.control.SelectionMode;
> import javafx.scene.layout.BorderPane;
> import javafx.scene.layout.VBox;
> import javafx.stage.Stage;
>
> public class SelectListViewTest extends Application {
> final int ROW_COUNT = 70_000;
> //final int ROW_COUNT = 400_000;
> //final int ROW_COUNT = 10_000_000;
> //final int ROW_COUNT = 7_000;
>
> @Override
> public void start(Stage stage) {
> ListView listView = new ListView<>();
> listView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
>
>
> ObservableList items = listView.getItems();
> for(int i=0; i String rec = String.valueOf(i);
> items.add(rec);
> }
> BorderPane root = new BorderPane(listView);
> Button selectAll = new Button("selectAll");
> Button clearSelection = new Button("clearSelection");
> Button selectToStart = new Button("selectToStart");
> Button selectToEnd = new Button("selectToEnd");
> Button selectPrevious = new Button("selectPrevious");
> Button selectNext= new Button("selectNext");
>
> selectAll.setFocusTraversable(true);
> clearSelection.setFocusTraversable(true);
> selectToStart.setFocusTraversable(true);
> selectToEnd.setFocusTraversable(true);
> selectPrevious.setFocusTraversable(true);
> selectNext.setFocusTraversable(true);
>
> root.setRight(new VBox(6, selectAll, selectToStart, selectToEnd,
> selectPrevious, selectNext, clearSelection));
> stage.setScene(new Scene(root, 600, 600));
>
> selectAll.setOnAction((e)->selectAll(listView));
> clearSelection.setOnAction((e)->clearSelection(listView));
> selectToStart.setOnAction((e)->selectToStart(listView));
> selectToEnd.setOnAction((e)->selectToLast(listView));
> selectPrevious.setOnAction((e)->selectPrevious(listView));
> selectNext.setOnAction((e)->selectNext(listView));
>
> stage.show();
> }
>
> private void selectAll(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectAll();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void clearSelection(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().clearSelection();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToStart(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectRange(0,
> listView.getSelectionModel().getSelectedIndex());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToLast(ListView listView) {
> long t = System.currentTimeMillis();
>
> listView.getSelectionModel().selectRange(listView.getSelectionModel().getSelectedIndex(),
> listView.getItems().size());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectPrevious(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectPrevious();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectNext(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectNext();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> public static void main(String[] args) {
> Application.launch(args);
> }
> }
>
>
> ``` SelectTableViewTest.java
> import javafx.application.Application;
> import
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Wed, 26 Feb 2020 07:37:06 GMT, yosbits
wrote:
> https://bugs.openjdk.java.net/browse/JDK-8197991
>
> The performance of the selectAll and selectRange methods of the
> MultiSelectionModel class has been greatly improved.
>
> This greatly improves the response when selecting multiple items in ListView
> and TableView.
>
> However, in TreeView / TreeTableView, the improvement effect is hidden mainly
> due to the design problem of the cache of TreeUtil.getTreeItem ().
>
> Reference value of the number of data that can be handled within 3 seconds of
> processing time (before-> after)
>
> ListView
> * selectAll: 400_000-> 10_000_000
> * selectRange: 7_000-> 70_000
>
> TableView
> * selectAll: 8_000-> 700_000
> * selectRange: 7_000-> 50_000
>
>
> You can see performance improvements in the following applications:
>
>
> ``` SelectListViewTest.java
> import javafx.application.Application;
> import javafx.collections.ObservableList;
> import javafx.scene.Scene;
> import javafx.scene.control.Button;
> import javafx.scene.control.ListView;
> import javafx.scene.control.SelectionMode;
> import javafx.scene.layout.BorderPane;
> import javafx.scene.layout.VBox;
> import javafx.stage.Stage;
>
> public class SelectListViewTest extends Application {
> final int ROW_COUNT = 70_000;
> //final int ROW_COUNT = 400_000;
> //final int ROW_COUNT = 10_000_000;
> //final int ROW_COUNT = 7_000;
>
> @Override
> public void start(Stage stage) {
> ListView listView = new ListView<>();
> listView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
>
>
> ObservableList items = listView.getItems();
> for(int i=0; i String rec = String.valueOf(i);
> items.add(rec);
> }
> BorderPane root = new BorderPane(listView);
> Button selectAll = new Button("selectAll");
> Button clearSelection = new Button("clearSelection");
> Button selectToStart = new Button("selectToStart");
> Button selectToEnd = new Button("selectToEnd");
> Button selectPrevious = new Button("selectPrevious");
> Button selectNext= new Button("selectNext");
>
> selectAll.setFocusTraversable(true);
> clearSelection.setFocusTraversable(true);
> selectToStart.setFocusTraversable(true);
> selectToEnd.setFocusTraversable(true);
> selectPrevious.setFocusTraversable(true);
> selectNext.setFocusTraversable(true);
>
> root.setRight(new VBox(6, selectAll, selectToStart, selectToEnd,
> selectPrevious, selectNext, clearSelection));
> stage.setScene(new Scene(root, 600, 600));
>
> selectAll.setOnAction((e)->selectAll(listView));
> clearSelection.setOnAction((e)->clearSelection(listView));
> selectToStart.setOnAction((e)->selectToStart(listView));
> selectToEnd.setOnAction((e)->selectToLast(listView));
> selectPrevious.setOnAction((e)->selectPrevious(listView));
> selectNext.setOnAction((e)->selectNext(listView));
>
> stage.show();
> }
>
> private void selectAll(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectAll();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void clearSelection(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().clearSelection();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToStart(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectRange(0,
> listView.getSelectionModel().getSelectedIndex());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToLast(ListView listView) {
> long t = System.currentTimeMillis();
>
> listView.getSelectionModel().selectRange(listView.getSelectionModel().getSelectedIndex(),
> listView.getItems().size());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectPrevious(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectPrevious();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectNext(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectNext();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> public static void main(String[] args) {
> Application.launch(args);
> }
> }
>
>
> ``` SelectTableViewTest.java
> import javafx.application.Application;
> import
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Wed, 26 Feb 2020 07:37:06 GMT, yosbits
wrote:
> https://bugs.openjdk.java.net/browse/JDK-8197991
>
> The performance of the selectAll and selectRange methods of the
> MultiSelectionModel class has been greatly improved.
>
> This greatly improves the response when selecting multiple items in ListView
> and TableView.
>
> However, in TreeView / TreeTableView, the improvement effect is hidden mainly
> due to the design problem of the cache of TreeUtil.getTreeItem ().
>
> Reference value of the number of data that can be handled within 3 seconds of
> processing time (before-> after)
>
> ListView
> * selectAll: 400_000-> 10_000_000
> * selectRange: 7_000-> 70_000
>
> TableView
> * selectAll: 8_000-> 700_000
> * selectRange: 7_000-> 50_000
>
>
> You can see performance improvements in the following applications:
>
>
> ``` SelectListViewTest.java
> import javafx.application.Application;
> import javafx.collections.ObservableList;
> import javafx.scene.Scene;
> import javafx.scene.control.Button;
> import javafx.scene.control.ListView;
> import javafx.scene.control.SelectionMode;
> import javafx.scene.layout.BorderPane;
> import javafx.scene.layout.VBox;
> import javafx.stage.Stage;
>
> public class SelectListViewTest extends Application {
> final int ROW_COUNT = 70_000;
> //final int ROW_COUNT = 400_000;
> //final int ROW_COUNT = 10_000_000;
> //final int ROW_COUNT = 7_000;
>
> @Override
> public void start(Stage stage) {
> ListView listView = new ListView<>();
> listView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
>
>
> ObservableList items = listView.getItems();
> for(int i=0; i String rec = String.valueOf(i);
> items.add(rec);
> }
> BorderPane root = new BorderPane(listView);
> Button selectAll = new Button("selectAll");
> Button clearSelection = new Button("clearSelection");
> Button selectToStart = new Button("selectToStart");
> Button selectToEnd = new Button("selectToEnd");
> Button selectPrevious = new Button("selectPrevious");
> Button selectNext= new Button("selectNext");
>
> selectAll.setFocusTraversable(true);
> clearSelection.setFocusTraversable(true);
> selectToStart.setFocusTraversable(true);
> selectToEnd.setFocusTraversable(true);
> selectPrevious.setFocusTraversable(true);
> selectNext.setFocusTraversable(true);
>
> root.setRight(new VBox(6, selectAll, selectToStart, selectToEnd,
> selectPrevious, selectNext, clearSelection));
> stage.setScene(new Scene(root, 600, 600));
>
> selectAll.setOnAction((e)->selectAll(listView));
> clearSelection.setOnAction((e)->clearSelection(listView));
> selectToStart.setOnAction((e)->selectToStart(listView));
> selectToEnd.setOnAction((e)->selectToLast(listView));
> selectPrevious.setOnAction((e)->selectPrevious(listView));
> selectNext.setOnAction((e)->selectNext(listView));
>
> stage.show();
> }
>
> private void selectAll(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectAll();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void clearSelection(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().clearSelection();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToStart(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectRange(0,
> listView.getSelectionModel().getSelectedIndex());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToLast(ListView listView) {
> long t = System.currentTimeMillis();
>
> listView.getSelectionModel().selectRange(listView.getSelectionModel().getSelectedIndex(),
> listView.getItems().size());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectPrevious(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectPrevious();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectNext(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectNext();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> public static void main(String[] args) {
> Application.launch(args);
> }
> }
>
>
> ``` SelectTableViewTest.java
> import javafx.application.Application;
> import
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Mon, 14 Sep 2020 10:03:59 GMT, Ajit Ghaisas wrote:
> Can you please provide an automated test along with your fix?
Automated performance testing should be distinguished from regular testing
tasks, as it extends the build time.
Is there a task name for that in gradle?
Also, didn't you exclude performance testing from automated testing (or Unit
Test)?
Or, if you want to maintain this test in a repository, you need to tell me the
directory where it is stored.
The reviewer didn't point out that the
There's a little bit of wastage left in the toArray(), so I'm going to push a
modified version.
> modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java
> line 861:
>
>> 859: }
>> 860:
>> 861: /** Returns number of true bits in BitSet */
>
> Method description and work done by it is no more matching. Can you please
> update the comment?
This comment is correct. this.size is the cache.
> modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java
> line 899:
>
>> 897: for (;;) {
>> 898: index = bitset.nextSetBit(index + 1);
>> 899: if (index < 0) {
>
> As we are checking for nextSetBit, shouldn't we be checking for overflow
> rather than underflow?
> Refer -
> [javadoc](https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/util/BitSet.html#nextSetBit(int))
Since it cannot be loaded with a smaller number of items than Integer.MAX_VALUE
(it looks like it freezes), overflow does not occur in the actual usage
environment.
-
PR: https://git.openjdk.java.net/jfx/pull/127
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Wed, 26 Feb 2020 07:37:06 GMT, yosbits
wrote:
> https://bugs.openjdk.java.net/browse/JDK-8197991
>
> The performance of the selectAll and selectRange methods of the
> MultiSelectionModel class has been greatly improved.
>
> This greatly improves the response when selecting multiple items in ListView
> and TableView.
>
> However, in TreeView / TreeTableView, the improvement effect is hidden mainly
> due to the design problem of the cache of TreeUtil.getTreeItem ().
>
> Reference value of the number of data that can be handled within 3 seconds of
> processing time (before-> after)
>
> ListView
> * selectAll: 400_000-> 10_000_000
> * selectRange: 7_000-> 70_000
>
> TableView
> * selectAll: 8_000-> 700_000
> * selectRange: 7_000-> 50_000
>
>
> You can see performance improvements in the following applications:
>
>
> ``` SelectListViewTest.java
> import javafx.application.Application;
> import javafx.collections.ObservableList;
> import javafx.scene.Scene;
> import javafx.scene.control.Button;
> import javafx.scene.control.ListView;
> import javafx.scene.control.SelectionMode;
> import javafx.scene.layout.BorderPane;
> import javafx.scene.layout.VBox;
> import javafx.stage.Stage;
>
> public class SelectListViewTest extends Application {
> final int ROW_COUNT = 70_000;
> //final int ROW_COUNT = 400_000;
> //final int ROW_COUNT = 10_000_000;
> //final int ROW_COUNT = 7_000;
>
> @Override
> public void start(Stage stage) {
> ListView listView = new ListView<>();
> listView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
>
>
> ObservableList items = listView.getItems();
> for(int i=0; i String rec = String.valueOf(i);
> items.add(rec);
> }
> BorderPane root = new BorderPane(listView);
> Button selectAll = new Button("selectAll");
> Button clearSelection = new Button("clearSelection");
> Button selectToStart = new Button("selectToStart");
> Button selectToEnd = new Button("selectToEnd");
> Button selectPrevious = new Button("selectPrevious");
> Button selectNext= new Button("selectNext");
>
> selectAll.setFocusTraversable(true);
> clearSelection.setFocusTraversable(true);
> selectToStart.setFocusTraversable(true);
> selectToEnd.setFocusTraversable(true);
> selectPrevious.setFocusTraversable(true);
> selectNext.setFocusTraversable(true);
>
> root.setRight(new VBox(6, selectAll, selectToStart, selectToEnd,
> selectPrevious, selectNext, clearSelection));
> stage.setScene(new Scene(root, 600, 600));
>
> selectAll.setOnAction((e)->selectAll(listView));
> clearSelection.setOnAction((e)->clearSelection(listView));
> selectToStart.setOnAction((e)->selectToStart(listView));
> selectToEnd.setOnAction((e)->selectToLast(listView));
> selectPrevious.setOnAction((e)->selectPrevious(listView));
> selectNext.setOnAction((e)->selectNext(listView));
>
> stage.show();
> }
>
> private void selectAll(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectAll();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void clearSelection(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().clearSelection();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToStart(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectRange(0,
> listView.getSelectionModel().getSelectedIndex());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToLast(ListView listView) {
> long t = System.currentTimeMillis();
>
> listView.getSelectionModel().selectRange(listView.getSelectionModel().getSelectedIndex(),
> listView.getItems().size());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectPrevious(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectPrevious();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectNext(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectNext();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> public static void main(String[] args) {
> Application.launch(args);
> }
> }
>
>
> ``` SelectTableViewTest.java
> import javafx.application.Application;
> import
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Wed, 26 Feb 2020 07:37:06 GMT, yosbits
wrote:
> https://bugs.openjdk.java.net/browse/JDK-8197991
>
> The performance of the selectAll and selectRange methods of the
> MultiSelectionModel class has been greatly improved.
>
> This greatly improves the response when selecting multiple items in ListView
> and TableView.
>
> However, in TreeView / TreeTableView, the improvement effect is hidden mainly
> due to the design problem of the cache of TreeUtil.getTreeItem ().
>
> Reference value of the number of data that can be handled within 3 seconds of
> processing time (before-> after)
>
> ListView
> * selectAll: 400_000-> 10_000_000
> * selectRange: 7_000-> 70_000
>
> TableView
> * selectAll: 8_000-> 700_000
> * selectRange: 7_000-> 50_000
>
>
> You can see performance improvements in the following applications:
>
>
> ``` SelectListViewTest.java
> import javafx.application.Application;
> import javafx.collections.ObservableList;
> import javafx.scene.Scene;
> import javafx.scene.control.Button;
> import javafx.scene.control.ListView;
> import javafx.scene.control.SelectionMode;
> import javafx.scene.layout.BorderPane;
> import javafx.scene.layout.VBox;
> import javafx.stage.Stage;
>
> public class SelectListViewTest extends Application {
> final int ROW_COUNT = 70_000;
> //final int ROW_COUNT = 400_000;
> //final int ROW_COUNT = 10_000_000;
> //final int ROW_COUNT = 7_000;
>
> @Override
> public void start(Stage stage) {
> ListView listView = new ListView<>();
> listView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
>
>
> ObservableList items = listView.getItems();
> for(int i=0; i String rec = String.valueOf(i);
> items.add(rec);
> }
> BorderPane root = new BorderPane(listView);
> Button selectAll = new Button("selectAll");
> Button clearSelection = new Button("clearSelection");
> Button selectToStart = new Button("selectToStart");
> Button selectToEnd = new Button("selectToEnd");
> Button selectPrevious = new Button("selectPrevious");
> Button selectNext= new Button("selectNext");
>
> selectAll.setFocusTraversable(true);
> clearSelection.setFocusTraversable(true);
> selectToStart.setFocusTraversable(true);
> selectToEnd.setFocusTraversable(true);
> selectPrevious.setFocusTraversable(true);
> selectNext.setFocusTraversable(true);
>
> root.setRight(new VBox(6, selectAll, selectToStart, selectToEnd,
> selectPrevious, selectNext, clearSelection));
> stage.setScene(new Scene(root, 600, 600));
>
> selectAll.setOnAction((e)->selectAll(listView));
> clearSelection.setOnAction((e)->clearSelection(listView));
> selectToStart.setOnAction((e)->selectToStart(listView));
> selectToEnd.setOnAction((e)->selectToLast(listView));
> selectPrevious.setOnAction((e)->selectPrevious(listView));
> selectNext.setOnAction((e)->selectNext(listView));
>
> stage.show();
> }
>
> private void selectAll(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectAll();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void clearSelection(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().clearSelection();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToStart(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectRange(0,
> listView.getSelectionModel().getSelectedIndex());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToLast(ListView listView) {
> long t = System.currentTimeMillis();
>
> listView.getSelectionModel().selectRange(listView.getSelectionModel().getSelectedIndex(),
> listView.getItems().size());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectPrevious(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectPrevious();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectNext(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectNext();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> public static void main(String[] args) {
> Application.launch(args);
> }
> }
>
>
> ``` SelectTableViewTest.java
> import javafx.application.Application;
> import
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Wed, 26 Feb 2020 07:37:06 GMT, yosbits
wrote:
> https://bugs.openjdk.java.net/browse/JDK-8197991
>
> The performance of the selectAll and selectRange methods of the
> MultiSelectionModel class has been greatly improved.
>
> This greatly improves the response when selecting multiple items in ListView
> and TableView.
>
> However, in TreeView / TreeTableView, the improvement effect is hidden mainly
> due to the design problem of the cache of TreeUtil.getTreeItem ().
>
> Reference value of the number of data that can be handled within 3 seconds of
> processing time (before-> after)
>
> ListView
> * selectAll: 400_000-> 10_000_000
> * selectRange: 7_000-> 70_000
>
> TableView
> * selectAll: 8_000-> 700_000
> * selectRange: 7_000-> 50_000
>
>
> You can see performance improvements in the following applications:
>
>
> ``` SelectListViewTest.java
> import javafx.application.Application;
> import javafx.collections.ObservableList;
> import javafx.scene.Scene;
> import javafx.scene.control.Button;
> import javafx.scene.control.ListView;
> import javafx.scene.control.SelectionMode;
> import javafx.scene.layout.BorderPane;
> import javafx.scene.layout.VBox;
> import javafx.stage.Stage;
>
> public class SelectListViewTest extends Application {
> final int ROW_COUNT = 70_000;
> //final int ROW_COUNT = 400_000;
> //final int ROW_COUNT = 10_000_000;
> //final int ROW_COUNT = 7_000;
>
> @Override
> public void start(Stage stage) {
> ListView listView = new ListView<>();
> listView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
>
>
> ObservableList items = listView.getItems();
> for(int i=0; i String rec = String.valueOf(i);
> items.add(rec);
> }
> BorderPane root = new BorderPane(listView);
> Button selectAll = new Button("selectAll");
> Button clearSelection = new Button("clearSelection");
> Button selectToStart = new Button("selectToStart");
> Button selectToEnd = new Button("selectToEnd");
> Button selectPrevious = new Button("selectPrevious");
> Button selectNext= new Button("selectNext");
>
> selectAll.setFocusTraversable(true);
> clearSelection.setFocusTraversable(true);
> selectToStart.setFocusTraversable(true);
> selectToEnd.setFocusTraversable(true);
> selectPrevious.setFocusTraversable(true);
> selectNext.setFocusTraversable(true);
>
> root.setRight(new VBox(6, selectAll, selectToStart, selectToEnd,
> selectPrevious, selectNext, clearSelection));
> stage.setScene(new Scene(root, 600, 600));
>
> selectAll.setOnAction((e)->selectAll(listView));
> clearSelection.setOnAction((e)->clearSelection(listView));
> selectToStart.setOnAction((e)->selectToStart(listView));
> selectToEnd.setOnAction((e)->selectToLast(listView));
> selectPrevious.setOnAction((e)->selectPrevious(listView));
> selectNext.setOnAction((e)->selectNext(listView));
>
> stage.show();
> }
>
> private void selectAll(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectAll();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void clearSelection(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().clearSelection();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToStart(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectRange(0,
> listView.getSelectionModel().getSelectedIndex());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToLast(ListView listView) {
> long t = System.currentTimeMillis();
>
> listView.getSelectionModel().selectRange(listView.getSelectionModel().getSelectedIndex(),
> listView.getItems().size());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectPrevious(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectPrevious();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectNext(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectNext();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> public static void main(String[] args) {
> Application.launch(args);
> }
> }
>
>
> ``` SelectTableViewTest.java
> import javafx.application.Application;
> import
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Tue, 29 Dec 2020 09:50:42 GMT, Jose Pereda wrote:
>> I commented.
>
> I've run SelectListView and SelectTableView tests and both run fine. As for
> the SelectTableView test, with 700_000 rows, when there is no selection,
> pressing SelectToEnd takes around 3.1 seconds, as expected. However, if there
> is one single row selected, after pressing SelectToEnd, the selection doesn't
> complete, and I have to abort and quit the app.
>
> Instead of:
> tableView.getSelectionModel().selectRange(tableView.getSelectionModel().getFocusedIndex(),
> tableView.getItems().size());
> this:
> int focusedIndex = tableView.getSelectionModel().getFocusedIndex();
> tableView.getSelectionModel().clearSelection();
> tableView.getSelectionModel().selectRange(focusedIndex,
> tableView.getItems().size());
> seems to work and times are again around 3 seconds or less.
>
> Maybe you can check why that is happening?
>
> And about the test discussion above, I understand that running the included
> tests as part of the automated test wouldn't make much sense (as these can
> take too long). However, maybe these could be added as unit tests with a
> reduced number of item/rows. Instead of measuring performance (selection
> times would depend on each machine), I'd say you could simply verify that
> selection works as expected.
>
> While most of the changes are just about caching `size()`, the new
> implementation of `MultipleSelectionModelBase::indexOf` adds some new code
> that should be tested, as part of the unit tests, again not for performance,
> but to assert it returns the expected values.
As an overview, the new implementation can handle selectRange up to 50_000
records. This assumes general manual operation. However, after clearing the
selection (a rare operation in manual operation), it is as efficient as
selectAll. However, this is a two-time change.
The response difference is caused by the next conditional branch (size == 0) of
SortedList. This will be the next hotspot to improve as seen by this
improvement.
I can't include it in this PR because I don't have time to work.
I haven't tried it, but if I change the per-record insertToMapping call of this
method to per-range setAllToMapping, the performance seems to be around
selectAll.
javafx.collections.transformation.SortedList.java
SortedList.java
private void addRemove(Change c) {
if (c.getFrom() == 0 && c.getRemovedSize() == size) {
removeAllFromMapping();
} else {
for (int i = 0, sz = c.getRemovedSize(); i < sz; ++i) {
removeFromMapping(c.getFrom(), c.getRemoved().get(i));
}
}
if (size == 0) {
setAllToMapping(c.getList(), c.getTo()); // This is basically
equivalent to getAddedSubList
// as size is 0, only
valid "from" is also 0
} else {
for (int i = c.getFrom(), to = c.getTo(); i < to; ++i) {
insertToMapping(c.getList().get(i), i);
}
}
}
-
PR: https://git.openjdk.java.net/jfx/pull/127
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Tue, 22 Sep 2020 09:14:31 GMT, yosbits wrote: >> yosbits has refreshed the contents of this pull request, and previous >> commits have been removed. The incremental views will show differences >> compared to the previous content of the PR. > > I commented. I've run SelectListView and SelectTableView tests and both run fine. As for the SelectTableView test, with 700_000 rows, when there is no selection, pressing SelectToEnd takes around 3.1 seconds, as expected. However, if there is one single row selected, after pressing SelectToEnd, the selection doesn't complete, and I have to abort and quit the app. Instead of: tableView.getSelectionModel().selectRange(tableView.getSelectionModel().getFocusedIndex(), tableView.getItems().size()); this: int focusedIndex = tableView.getSelectionModel().getFocusedIndex(); tableView.getSelectionModel().clearSelection(); tableView.getSelectionModel().selectRange(focusedIndex, tableView.getItems().size()); seems to work and times are again around 3 seconds or less. Maybe you can check why that is happening? And about the test discussion above, I understand that running the included tests as part of the automated test wouldn't make much sense (as these can take too long). However, maybe these could be added as unit tests with a reduced number of item/rows. Instead of measuring performance (selection times would depend on each machine), I'd say you could simply verify that selection works as expected. While most of the changes are just about caching `size()`, the new implementation of `MultipleSelectionModelBase::indexOf` adds some new code that should be tested, as part of the unit tests, again not for performance, but to assert it returns the expected values. - PR: https://git.openjdk.java.net/jfx/pull/127
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Tue, 22 Sep 2020 09:04:15 GMT, yosbits
wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8197991
>>
>> The performance of the selectAll and selectRange methods of the
>> MultiSelectionModel class has been greatly improved.
>>
>> This greatly improves the response when selecting multiple items in ListView
>> and TableView.
>>
>> However, in TreeView / TreeTableView, the improvement effect is hidden
>> mainly due to the design problem of the cache of
>> TreeUtil.getTreeItem ().
>> Reference value of the number of data that can be handled within 3 seconds
>> of processing time (before-> after)
>>
>> ListView
>> * selectAll: 400_000-> 10_000_000
>> * selectRange: 7_000-> 70_000
>>
>> TableView
>> * selectAll: 8_000-> 700_000
>> * selectRange: 7_000-> 50_000
>>
>>
>> You can see performance improvements in the following applications:
>>
>>
>> SelectListViewTest.java
>> import javafx.application.Application;
>> import javafx.collections.ObservableList;
>> import javafx.scene.Scene;
>> import javafx.scene.control.Button;
>> import javafx.scene.control.ListView;
>> import javafx.scene.control.SelectionMode;
>> import javafx.scene.layout.BorderPane;
>> import javafx.scene.layout.VBox;
>> import javafx.stage.Stage;
>>
>> public class SelectListViewTest extends Application {
>> final int ROW_COUNT = 70_000;
>> // final int ROW_COUNT = 400_000;
>> // final int ROW_COUNT = 10_000_000;
>> // final int ROW_COUNT = 7_000;
>>
>> @Override
>> public void start(Stage stage) {
>> ListView listView = new ListView<>();
>> listView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
>>
>>
>> ObservableList items = listView.getItems();
>> for(int i=0; i> String rec = String.valueOf(i);
>> items.add(rec);
>> }
>> BorderPane root = new BorderPane(listView);
>> Button selectAll = new Button("selectAll");
>> Button clearSelection = new Button("clearSelection");
>> Button selectToStart = new Button("selectToStart");
>> Button selectToEnd = new Button("selectToEnd");
>> Button selectPrevious = new Button("selectPrevious");
>> Button selectNext= new Button("selectNext");
>>
>> selectAll.setFocusTraversable(true);
>> clearSelection.setFocusTraversable(true);
>> selectToStart.setFocusTraversable(true);
>> selectToEnd.setFocusTraversable(true);
>> selectPrevious.setFocusTraversable(true);
>> selectNext.setFocusTraversable(true);
>>
>> root.setRight(new VBox(6, selectAll, selectToStart, selectToEnd,
>> selectPrevious, selectNext, clearSelection));
>> stage.setScene(new Scene(root, 600, 600));
>>
>> selectAll.setOnAction((e)->selectAll(listView));
>> clearSelection.setOnAction((e)->clearSelection(listView));
>> selectToStart.setOnAction((e)->selectToStart(listView));
>> selectToEnd.setOnAction((e)->selectToLast(listView));
>> selectPrevious.setOnAction((e)->selectPrevious(listView));
>> selectNext.setOnAction((e)->selectNext(listView));
>>
>> stage.show();
>> }
>>
>> private void selectAll(ListView listView) {
>> long t = System.currentTimeMillis();
>> listView.getSelectionModel().selectAll();
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>> private void clearSelection(ListView listView) {
>> long t = System.currentTimeMillis();
>> listView.getSelectionModel().clearSelection();
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>> private void selectToStart(ListView listView) {
>> long t = System.currentTimeMillis();
>> listView.getSelectionModel().selectRange(0,
>> listView.getSelectionModel().getSelectedIndex());
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>> private void selectToLast(ListView listView) {
>> long t = System.currentTimeMillis();
>>
>> listView.getSelectionModel().selectRange(listView.getSelectionModel().getSelectedIndex(),
>> listView.getItems().size());
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>>
>> private void selectPrevious(ListView listView) {
>> long t = System.currentTimeMillis();
>> listView.getSelectionModel().selectPrevious();
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>>
>> private void selectNext(ListView listView) {
>> long t = System.currentTimeMillis();
>> listView.getSelectionModel().selectNext();
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>> public static void main(String[] args) {
>> Application.launch(args);
>> }
>> }
>>
>> SelectTableViewTest.java
>> import
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Mon, 14 Sep 2020 09:50:05 GMT, Ajit Ghaisas wrote:
>> yosbits has refreshed the contents of this pull request, and previous
>> commits have been removed. The incremental views
>> will show differences compared to the previous content of the PR.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java
> line 899:
>
>> 897: for (;;) {
>> 898: index = bitset.nextSetBit(index + 1);
>> 899: if (index < 0) {
>
> As we are checking for nextSetBit, shouldn't we be checking for overflow
> rather than underflow?
> Refer -
> [javadoc](https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/util/BitSet.html#nextSetBit(int))
Since it cannot be loaded with a smaller number of items than Integer.MAX_VALUE
(it looks like it freezes), overflow
does not occur in the actual usage environment.
-
PR: https://git.openjdk.java.net/jfx/pull/127
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
> https://bugs.openjdk.java.net/browse/JDK-8197991
>
> The performance of the selectAll and selectRange methods of the
> MultiSelectionModel class has been greatly improved.
>
> This greatly improves the response when selecting multiple items in ListView
> and TableView.
>
> However, in TreeView / TreeTableView, the improvement effect is hidden mainly
> due to the design problem of the cache of
> TreeUtil.getTreeItem ().
> Reference value of the number of data that can be handled within 3 seconds of
> processing time (before-> after)
>
> ListView
> * selectAll: 400_000-> 10_000_000
> * selectRange: 7_000-> 70_000
>
> TableView
> * selectAll: 8_000-> 700_000
> * selectRange: 7_000-> 50_000
>
>
> You can see performance improvements in the following applications:
>
>
> SelectListViewTest.java
> import javafx.application.Application;
> import javafx.collections.ObservableList;
> import javafx.scene.Scene;
> import javafx.scene.control.Button;
> import javafx.scene.control.ListView;
> import javafx.scene.control.SelectionMode;
> import javafx.scene.layout.BorderPane;
> import javafx.scene.layout.VBox;
> import javafx.stage.Stage;
>
> public class SelectListViewTest extends Application {
> final int ROW_COUNT = 70_000;
> //final int ROW_COUNT = 400_000;
> //final int ROW_COUNT = 10_000_000;
> //final int ROW_COUNT = 7_000;
>
> @Override
> public void start(Stage stage) {
> ListView listView = new ListView<>();
> listView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
>
>
> ObservableList items = listView.getItems();
> for(int i=0; i String rec = String.valueOf(i);
> items.add(rec);
> }
> BorderPane root = new BorderPane(listView);
> Button selectAll = new Button("selectAll");
> Button clearSelection = new Button("clearSelection");
> Button selectToStart = new Button("selectToStart");
> Button selectToEnd = new Button("selectToEnd");
> Button selectPrevious = new Button("selectPrevious");
> Button selectNext= new Button("selectNext");
>
> selectAll.setFocusTraversable(true);
> clearSelection.setFocusTraversable(true);
> selectToStart.setFocusTraversable(true);
> selectToEnd.setFocusTraversable(true);
> selectPrevious.setFocusTraversable(true);
> selectNext.setFocusTraversable(true);
>
> root.setRight(new VBox(6, selectAll, selectToStart, selectToEnd,
> selectPrevious, selectNext, clearSelection));
> stage.setScene(new Scene(root, 600, 600));
>
> selectAll.setOnAction((e)->selectAll(listView));
> clearSelection.setOnAction((e)->clearSelection(listView));
> selectToStart.setOnAction((e)->selectToStart(listView));
> selectToEnd.setOnAction((e)->selectToLast(listView));
> selectPrevious.setOnAction((e)->selectPrevious(listView));
> selectNext.setOnAction((e)->selectNext(listView));
>
> stage.show();
> }
>
> private void selectAll(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectAll();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void clearSelection(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().clearSelection();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToStart(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectRange(0,
> listView.getSelectionModel().getSelectedIndex());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToLast(ListView listView) {
> long t = System.currentTimeMillis();
>
> listView.getSelectionModel().selectRange(listView.getSelectionModel().getSelectedIndex(),
> listView.getItems().size());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectPrevious(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectPrevious();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectNext(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectNext();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> public static void main(String[] args) {
> Application.launch(args);
> }
> }
>
> SelectTableViewTest.java
> import javafx.application.Application;
> import javafx.beans.property.SimpleStringProperty;
> import
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v3]
On Mon, 14 Sep 2020 09:53:25 GMT, Ajit Ghaisas wrote: >> yosbits has refreshed the contents of this pull request, and previous >> commits have been removed. The incremental views >> will show differences compared to the previous content of the PR. The pull >> request contains one new commit since the >> last revision: >> 8197991: Fix multi-select performance issues > > modules/javafx.controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java > line 861: > >> 859: } >> 860: >> 861: /** Returns number of true bits in BitSet */ > > Method description and work done by it is no more matching. Can you please > update the comment? This comment is correct. this.size is the cache. - PR: https://git.openjdk.java.net/jfx/pull/127
Re: RFR: 8197991: Selecting many items in a TableView is very slow [v2]
> https://bugs.openjdk.java.net/browse/JDK-8197991
>
> The performance of the selectAll and selectRange methods of the
> MultiSelectionModel class has been greatly improved.
>
> This greatly improves the response when selecting multiple items in ListView
> and TableView.
>
> However, in TreeView / TreeTableView, the improvement effect is hidden mainly
> due to the design problem of the cache of
> TreeUtil.getTreeItem ().
> Reference value of the number of data that can be handled within 3 seconds of
> processing time (before-> after)
>
> ListView
> * selectAll: 400_000-> 10_000_000
> * selectRange: 7_000-> 70_000
>
> TableView
> * selectAll: 8_000-> 700_000
> * selectRange: 7_000-> 50_000
>
>
> You can see performance improvements in the following applications:
>
>
> SelectListViewTest.java
> import javafx.application.Application;
> import javafx.collections.ObservableList;
> import javafx.scene.Scene;
> import javafx.scene.control.Button;
> import javafx.scene.control.ListView;
> import javafx.scene.control.SelectionMode;
> import javafx.scene.layout.BorderPane;
> import javafx.scene.layout.VBox;
> import javafx.stage.Stage;
>
> public class SelectListViewTest extends Application {
> final int ROW_COUNT = 70_000;
> //final int ROW_COUNT = 400_000;
> //final int ROW_COUNT = 10_000_000;
> //final int ROW_COUNT = 7_000;
>
> @Override
> public void start(Stage stage) {
> ListView listView = new ListView<>();
> listView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
>
>
> ObservableList items = listView.getItems();
> for(int i=0; i String rec = String.valueOf(i);
> items.add(rec);
> }
> BorderPane root = new BorderPane(listView);
> Button selectAll = new Button("selectAll");
> Button clearSelection = new Button("clearSelection");
> Button selectToStart = new Button("selectToStart");
> Button selectToEnd = new Button("selectToEnd");
> Button selectPrevious = new Button("selectPrevious");
> Button selectNext= new Button("selectNext");
>
> selectAll.setFocusTraversable(true);
> clearSelection.setFocusTraversable(true);
> selectToStart.setFocusTraversable(true);
> selectToEnd.setFocusTraversable(true);
> selectPrevious.setFocusTraversable(true);
> selectNext.setFocusTraversable(true);
>
> root.setRight(new VBox(6, selectAll, selectToStart, selectToEnd,
> selectPrevious, selectNext, clearSelection));
> stage.setScene(new Scene(root, 600, 600));
>
> selectAll.setOnAction((e)->selectAll(listView));
> clearSelection.setOnAction((e)->clearSelection(listView));
> selectToStart.setOnAction((e)->selectToStart(listView));
> selectToEnd.setOnAction((e)->selectToLast(listView));
> selectPrevious.setOnAction((e)->selectPrevious(listView));
> selectNext.setOnAction((e)->selectNext(listView));
>
> stage.show();
> }
>
> private void selectAll(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectAll();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void clearSelection(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().clearSelection();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToStart(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectRange(0,
> listView.getSelectionModel().getSelectedIndex());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToLast(ListView listView) {
> long t = System.currentTimeMillis();
>
> listView.getSelectionModel().selectRange(listView.getSelectionModel().getSelectedIndex(),
> listView.getItems().size());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectPrevious(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectPrevious();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectNext(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectNext();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> public static void main(String[] args) {
> Application.launch(args);
> }
> }
>
> SelectTableViewTest.java
> import javafx.application.Application;
> import javafx.beans.property.SimpleStringProperty;
> import
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Mon, 14 Sep 2020 10:03:59 GMT, Ajit Ghaisas wrote: >> Any progress with having this merged? > > This is a very good attempt to improve selection performance. I have few > review comments. > I ran the manual test that you have provided. It does show the improvement. > Can you please provide an automated test along with your fix? The reviewer didn't point out that the There's a little bit of wastage left in the toArray(), so I'm going to push a modified version. - PR: https://git.openjdk.java.net/jfx/pull/127
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Wed, 26 Feb 2020 07:37:06 GMT, yosbits
wrote:
> https://bugs.openjdk.java.net/browse/JDK-8197991
>
> The performance of the selectAll and selectRange methods of the
> MultiSelectionModel class has been greatly improved.
>
> This greatly improves the response when selecting multiple items in ListView
> and TableView.
>
> However, in TreeView / TreeTableView, the improvement effect is hidden mainly
> due to the design problem of the cache of
> TreeUtil.getTreeItem ().
> Reference value of the number of data that can be handled within 3 seconds of
> processing time (before-> after)
>
> ListView
> * selectAll: 400_000-> 10_000_000
> * selectRange: 7_000-> 70_000
>
> TableView
> * selectAll: 8_000-> 700_000
> * selectRange: 7_000-> 50_000
>
>
> You can see performance improvements in the following applications:
>
>
> SelectListViewTest.java
> import javafx.application.Application;
> import javafx.collections.ObservableList;
> import javafx.scene.Scene;
> import javafx.scene.control.Button;
> import javafx.scene.control.ListView;
> import javafx.scene.control.SelectionMode;
> import javafx.scene.layout.BorderPane;
> import javafx.scene.layout.VBox;
> import javafx.stage.Stage;
>
> public class SelectListViewTest extends Application {
> final int ROW_COUNT = 70_000;
> //final int ROW_COUNT = 400_000;
> //final int ROW_COUNT = 10_000_000;
> //final int ROW_COUNT = 7_000;
>
> @Override
> public void start(Stage stage) {
> ListView listView = new ListView<>();
> listView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
>
>
> ObservableList items = listView.getItems();
> for(int i=0; i String rec = String.valueOf(i);
> items.add(rec);
> }
> BorderPane root = new BorderPane(listView);
> Button selectAll = new Button("selectAll");
> Button clearSelection = new Button("clearSelection");
> Button selectToStart = new Button("selectToStart");
> Button selectToEnd = new Button("selectToEnd");
> Button selectPrevious = new Button("selectPrevious");
> Button selectNext= new Button("selectNext");
>
> selectAll.setFocusTraversable(true);
> clearSelection.setFocusTraversable(true);
> selectToStart.setFocusTraversable(true);
> selectToEnd.setFocusTraversable(true);
> selectPrevious.setFocusTraversable(true);
> selectNext.setFocusTraversable(true);
>
> root.setRight(new VBox(6, selectAll, selectToStart, selectToEnd,
> selectPrevious, selectNext, clearSelection));
> stage.setScene(new Scene(root, 600, 600));
>
> selectAll.setOnAction((e)->selectAll(listView));
> clearSelection.setOnAction((e)->clearSelection(listView));
> selectToStart.setOnAction((e)->selectToStart(listView));
> selectToEnd.setOnAction((e)->selectToLast(listView));
> selectPrevious.setOnAction((e)->selectPrevious(listView));
> selectNext.setOnAction((e)->selectNext(listView));
>
> stage.show();
> }
>
> private void selectAll(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectAll();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void clearSelection(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().clearSelection();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToStart(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectRange(0,
> listView.getSelectionModel().getSelectedIndex());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToLast(ListView listView) {
> long t = System.currentTimeMillis();
>
> listView.getSelectionModel().selectRange(listView.getSelectionModel().getSelectedIndex(),
> listView.getItems().size());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectPrevious(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectPrevious();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectNext(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectNext();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> public static void main(String[] args) {
> Application.launch(args);
> }
> }
>
> SelectTableViewTest.java
> import javafx.application.Application;
> import
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Mon, 7 Sep 2020 18:14:03 GMT, Timm wrote: >> @aghaisas can you also review this? > > Any progress with having this merged? This is a very good attempt to improve selection performance. I have few review comments. I ran the manual test that you have provided. It does show the improvement. Can you please provide an automated test along with your fix? - PR: https://git.openjdk.java.net/jfx/pull/127
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Tue, 21 Apr 2020 12:50:50 GMT, Kevin Rushforth 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. > > @aghaisas can you also review this? Any progress with having this merged? - PR: https://git.openjdk.java.net/jfx/pull/127
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Thu, 2 Apr 2020 17:37:26 GMT, Dalibor Topic wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8197991
>>
>> The performance of the selectAll and selectRange methods of the
>> MultiSelectionModel class has been greatly improved.
>>
>> This greatly improves the response when selecting multiple items in ListView
>> and TableView.
>>
>> However, in TreeView / TreeTableView, the improvement effect is hidden
>> mainly due to the design problem of the cache of
>> TreeUtil.getTreeItem ().
>> Reference value of the number of data that can be handled within 3 seconds
>> of processing time (before-> after)
>>
>> ListView
>> * selectAll: 400_000-> 10_000_000
>> * selectRange: 7_000-> 70_000
>>
>> TableView
>> * selectAll: 8_000-> 700_000
>> * selectRange: 7_000-> 50_000
>>
>>
>> You can see performance improvements in the following applications:
>>
>>
>> SelectListViewTest.java
>> import javafx.application.Application;
>> import javafx.collections.ObservableList;
>> import javafx.scene.Scene;
>> import javafx.scene.control.Button;
>> import javafx.scene.control.ListView;
>> import javafx.scene.control.SelectionMode;
>> import javafx.scene.layout.BorderPane;
>> import javafx.scene.layout.VBox;
>> import javafx.stage.Stage;
>>
>> public class SelectListViewTest extends Application {
>> final int ROW_COUNT = 70_000;
>> // final int ROW_COUNT = 400_000;
>> // final int ROW_COUNT = 10_000_000;
>> // final int ROW_COUNT = 7_000;
>>
>> @Override
>> public void start(Stage stage) {
>> ListView listView = new ListView<>();
>> listView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
>>
>>
>> ObservableList items = listView.getItems();
>> for(int i=0; i> String rec = String.valueOf(i);
>> items.add(rec);
>> }
>> BorderPane root = new BorderPane(listView);
>> Button selectAll = new Button("selectAll");
>> Button clearSelection = new Button("clearSelection");
>> Button selectToStart = new Button("selectToStart");
>> Button selectToEnd = new Button("selectToEnd");
>> Button selectPrevious = new Button("selectPrevious");
>> Button selectNext= new Button("selectNext");
>>
>> selectAll.setFocusTraversable(true);
>> clearSelection.setFocusTraversable(true);
>> selectToStart.setFocusTraversable(true);
>> selectToEnd.setFocusTraversable(true);
>> selectPrevious.setFocusTraversable(true);
>> selectNext.setFocusTraversable(true);
>>
>> root.setRight(new VBox(6, selectAll, selectToStart, selectToEnd,
>> selectPrevious, selectNext, clearSelection));
>> stage.setScene(new Scene(root, 600, 600));
>>
>> selectAll.setOnAction((e)->selectAll(listView));
>> clearSelection.setOnAction((e)->clearSelection(listView));
>> selectToStart.setOnAction((e)->selectToStart(listView));
>> selectToEnd.setOnAction((e)->selectToLast(listView));
>> selectPrevious.setOnAction((e)->selectPrevious(listView));
>> selectNext.setOnAction((e)->selectNext(listView));
>>
>> stage.show();
>> }
>>
>> private void selectAll(ListView listView) {
>> long t = System.currentTimeMillis();
>> listView.getSelectionModel().selectAll();
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>> private void clearSelection(ListView listView) {
>> long t = System.currentTimeMillis();
>> listView.getSelectionModel().clearSelection();
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>> private void selectToStart(ListView listView) {
>> long t = System.currentTimeMillis();
>> listView.getSelectionModel().selectRange(0,
>> listView.getSelectionModel().getSelectedIndex());
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>> private void selectToLast(ListView listView) {
>> long t = System.currentTimeMillis();
>>
>> listView.getSelectionModel().selectRange(listView.getSelectionModel().getSelectedIndex(),
>> listView.getItems().size());
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>>
>> private void selectPrevious(ListView listView) {
>> long t = System.currentTimeMillis();
>> listView.getSelectionModel().selectPrevious();
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>>
>> private void selectNext(ListView listView) {
>> long t = System.currentTimeMillis();
>> listView.getSelectionModel().selectNext();
>> System.out.println("time:"+ (System.currentTimeMillis() - t));
>> }
>> public static void main(String[] args) {
>> Application.launch(args);
>> }
>> }
>>
>> SelectTableViewTest.java
>> import
Re: RFR: 8197991: Selecting many items in a TableView is very slow
On Wed, 26 Feb 2020 07:37:06 GMT, yosbits
wrote:
> https://bugs.openjdk.java.net/browse/JDK-8197991
>
> The performance of the selectAll and selectRange methods of the
> MultiSelectionModel class has been greatly improved.
>
> This greatly improves the response when selecting multiple items in ListView
> and TableView.
>
> However, in TreeView / TreeTableView, the improvement effect is hidden mainly
> due to the design problem of the cache of
> TreeUtil.getTreeItem ().
> Reference value of the number of data that can be handled within 3 seconds of
> processing time (before-> after)
>
> ListView
> * selectAll: 400_000-> 10_000_000
> * selectRange: 7_000-> 70_000
>
> TableView
> * selectAll: 8_000-> 700_000
> * selectRange: 7_000-> 50_000
>
>
> You can see performance improvements in the following applications:
>
>
> SelectListViewTest.java
> import javafx.application.Application;
> import javafx.collections.ObservableList;
> import javafx.scene.Scene;
> import javafx.scene.control.Button;
> import javafx.scene.control.ListView;
> import javafx.scene.control.SelectionMode;
> import javafx.scene.layout.BorderPane;
> import javafx.scene.layout.VBox;
> import javafx.stage.Stage;
>
> public class SelectListViewTest extends Application {
> final int ROW_COUNT = 70_000;
> //final int ROW_COUNT = 400_000;
> //final int ROW_COUNT = 10_000_000;
> //final int ROW_COUNT = 7_000;
>
> @Override
> public void start(Stage stage) {
> ListView listView = new ListView<>();
> listView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
>
>
> ObservableList items = listView.getItems();
> for(int i=0; i String rec = String.valueOf(i);
> items.add(rec);
> }
> BorderPane root = new BorderPane(listView);
> Button selectAll = new Button("selectAll");
> Button clearSelection = new Button("clearSelection");
> Button selectToStart = new Button("selectToStart");
> Button selectToEnd = new Button("selectToEnd");
> Button selectPrevious = new Button("selectPrevious");
> Button selectNext= new Button("selectNext");
>
> selectAll.setFocusTraversable(true);
> clearSelection.setFocusTraversable(true);
> selectToStart.setFocusTraversable(true);
> selectToEnd.setFocusTraversable(true);
> selectPrevious.setFocusTraversable(true);
> selectNext.setFocusTraversable(true);
>
> root.setRight(new VBox(6, selectAll, selectToStart, selectToEnd,
> selectPrevious, selectNext, clearSelection));
> stage.setScene(new Scene(root, 600, 600));
>
> selectAll.setOnAction((e)->selectAll(listView));
> clearSelection.setOnAction((e)->clearSelection(listView));
> selectToStart.setOnAction((e)->selectToStart(listView));
> selectToEnd.setOnAction((e)->selectToLast(listView));
> selectPrevious.setOnAction((e)->selectPrevious(listView));
> selectNext.setOnAction((e)->selectNext(listView));
>
> stage.show();
> }
>
> private void selectAll(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectAll();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void clearSelection(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().clearSelection();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToStart(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectRange(0,
> listView.getSelectionModel().getSelectedIndex());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> private void selectToLast(ListView listView) {
> long t = System.currentTimeMillis();
>
> listView.getSelectionModel().selectRange(listView.getSelectionModel().getSelectedIndex(),
> listView.getItems().size());
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectPrevious(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectPrevious();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
>
> private void selectNext(ListView listView) {
> long t = System.currentTimeMillis();
> listView.getSelectionModel().selectNext();
> System.out.println("time:"+ (System.currentTimeMillis() - t));
> }
> public static void main(String[] args) {
> Application.launch(args);
> }
> }
>
> SelectTableViewTest.java
> import javafx.application.Application;
> import
