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 [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 [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 [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 [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
