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&pr=673&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=673&range=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&pr=673&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=673&range=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: 8191995: Regression: DatePicker must commit on focusLost
On Wed, 24 Nov 2021 09:09:53 GMT, Marius Hanl wrote: > This PR fixes an issue where the `DatePicker` is not committing his value > when the focus is lost. > As the ticket also mentions, this is a regression which last worked on JavaFX > 8 and got broken by the refactoring: > [JDK-8150946](https://bugs.openjdk.java.net/browse/JDK-8150946) > > The fix is to provide the same api to the `DatePicker` which was introduced > by [JDK-8150946](https://bugs.openjdk.java.net/browse/JDK-8150946) for > `ComboBox` and `Spinner`. > > Note: While fixing this I found a possible bug which I tracked here: > [JDK-8277756](https://bugs.openjdk.java.net/browse/JDK-8277756) > -> When creating a `DatePicker` with the second constructor (with `LocalDate` > as parameter) two listener won't be added since they are only added at the > first constructor (That's also why I added the focusProperty listener in the > second constructor). I just finished the CSR. :) - PR: https://git.openjdk.java.net/jfx/pull/679
Integrated: 8276206: Rename TextBinding class to better express its purpose
On Wed, 24 Nov 2021 04:23:59 GMT, Michael Strauß wrote: > This PR renames `TextBinding` to `MnemonicInfo` and adds the following text > to its javadoc: > > /** > 33 +* Provides information about mnemonics contained within a string. This pull request has now been integrated. Changeset: fc3792d5 Author:Michael Strauß Committer: Jeanette Winzenburg URL: https://git.openjdk.java.net/jfx/commit/fc3792d5cb39cd3706f28953c3f97bdecc07bba6 Stats: 320 lines in 6 files changed: 146 ins; 146 del; 28 mod 8276206: Rename TextBinding class to better express its purpose Reviewed-by: kcr, fastegal - PR: https://git.openjdk.java.net/jfx/pull/678
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: 8276206: Rename TextBinding class to better express its purpose
On Wed, 24 Nov 2021 04:23:59 GMT, Michael Strauß wrote: > This PR renames `TextBinding` to `MnemonicInfo` and adds the following text > to its javadoc: > > /** > 33 +* Provides information about mnemonics contained within a string. looks good (modulo our disagreement on whether or not to do it at all :) - Marked as reviewed by fastegal (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/678
