Re: Possible approaches to JDK-8185886: Improve scrolling performance of TableView and TreeTableView
It seems clear now that we will need 3 different JBS issues for these
proposed performance enhancements. It's a holiday weekend coming up in
the US, so I can file the other two issues unless someone else gets to
it first. Unless there is a good reason to do otherwise, I propose:
The JBS Issue associated with PR #108 should be filed against javafx/base
The JBS Issue associated with PR #125 should be filed against
javafx/controls (or we can reuse JDK-8185886)
The JBS Issue associated with PR #185 should be filed against
javafx/scenegraph
Jose: Is you approach an alternative to one of the above or could it be
considered a 4th approach? If the latter, can you file a new JBS Issue
for that?
-- Kevin
On 9/2/2020 5:24 AM, Jeanette Winzenburg wrote:
Hi John,
thanks for the clarification :)
Hmm .. but then it's not really a PR against JDK-8185886 (scrolling
performance was always bad with many columns) but against - yet to be
reported - side-effect of JDK-8090322 which happens to detoriate
tableView performance even further (there might be other side-effects)?
-- Jeanette
Zitat von John Hendrikx :
The "dynamic update performance" performance issue we are seeing is a
regression from JDK-8090322. In this change the Node treeShowing
property was introduced. The JDK-8090322 warns in its description
about:
""" Considerations:
* This is too expensive to calculate for all nodes by default. So the
simplest way to provide it would be a special binding implementation
or a util class. Where you create a instance and pass in the node you
are interested in. It can then register listeners all the way up the
tree and listen to what it needs. """
The above comment is warning against the fact that registering
listeners for EACH Node on Window and Scene is going to be a
performance issue. As nodes can number in the 1000's or 10.000's,
that's a lot of listeners to store in a List data structure.
PR 185 targets this issue and implements the feature in JDK-8090322 in
the way that was suggested in the above comment, instead of how it
currently is implemented (registering listeners for every Node, just
in case someone needs the treeShowing property).
It's scope is not as broad as it would seem (because of a change in
Node). It effectively only makes a small change in two controls
(PopupWindow and ProgressIndicatorSkin).
--John
On 31/08/2020 16:54, Jeanette Winzenburg wrote:
Looking at the examples provided in 108/125: apart from both having
many
columns (> 300 makes them really nasty) they differ in
Table content:
125 - static data
108 - items are frequently modified (added)
Perceived performance:
125 - vertical scrolling: thumb/content lags behind mouse
108 - with enough columns, all interaction is sluggish (mouse, keys,
..), scrolling being just one of them
Both have examples, running those against the suggested fixes (with
108a
for Jose's approach)
125 - fixes its own example, does nothing for the other
108, 108a, 185 - improves its own example, does nothing for other
So we seem to have multiple issues that are (mostly) orthogonal: one
being the broken/missing horizontal virtualization (125), the other
related to dynamic update of table content (108, 108a, 185). We need to
solve both, the solution/s for one looks (mostly?) unrelated to the
solution to the other.
125 is the only one PR for its use-case, and it seems to do its job.
From those targeting the dynamic data update all except Jose's (not yet
formalized into a PR) approach feel too broad: table's reaction to
items
modifications is .. suboptimal in more than one aspect. Changing
overall
notification implementation to improve that, sounds like covering it
up.
-- Jeanette
Zitat von Kevin Rushforth :
Sorry for the badly formatted html. Here it is again.
I see some progress being made on the {Tree}TableView performance
issue. To summarize where I think we are:
There are currently 2 different approaches under review:
1. https://github.com/openjdk/jfx/pull/108 : optimization in
javafx.base to make removing listeners faster
2. https://github.com/openjdk/jfx/pull/125 : optimization in TableView
to reduce the number of add / removes
In addition, the following is a WIP PR that the author thinks could be
a 3rd approach:
3. https://github.com/openjdk/jfx/pull/185 : optimization in scene
graph to avoid install treeShowing listeners on Window and Scene for
all nodes
Jose has proposed a 4th approach as a comment to PR #108, and as I
understand it, he will propose a PR shortly.
4. Don't clear the list of children in a VirtualFlow when the number
of items changes.
So the first thing that is needed is to evaluate the approaches and
decide which one to pursue.
Options 1 and 3 are more broad in their scope, option #2 is more
targeted (to TableView), but within that area is a larger change.
Option #3 would remove the (internal) treeShowing property as a
generally available concept and only use it for controls lik
Integrated: 8202990: javafx webview css filter property with display scaling
On Tue, 11 Aug 2020 16:18:37 GMT, Bhawesh Choudhary wrote: > ImageJava.cpp ignores CompositeOperator parameter in drawImage function due > to which shadow was getting drawn on top of > actual image. apply given composite operator to graphics context before > drawing image to fix this issue. another issue > is into WCGraphicsPrismContext.java. while blending two layers, applying > state to the destination layer was missed due > to which image was not getting drawn with right scale in hidpi mode. apply > state to fix the issue. This pull request has now been integrated. Changeset: 3cb3ca84 Author:Bhawesh Choudhary Committer: Kevin Rushforth URL: https://git.openjdk.java.net/jfx/commit/3cb3ca84 Stats: 200 lines in 4 files changed: 0 ins; 199 del; 1 mod 8202990: javafx webview css filter property with display scaling Reviewed-by: kcr, ajoseph - PR: https://git.openjdk.java.net/jfx/pull/279
Re: RFR: 8202990: javafx webview css filter property with display scaling [v3]
On Thu, 3 Sep 2020 06:07:10 GMT, Bhawesh Choudhary wrote: >> ImageJava.cpp ignores CompositeOperator parameter in drawImage function due >> to which shadow was getting drawn on top of >> actual image. apply given composite operator to graphics context before >> drawing image to fix this issue. another issue >> is into WCGraphicsPrismContext.java. while blending two layers, applying >> state to the destination layer was missed due >> to which image was not getting drawn with right scale in hidpi mode. apply >> state to fix the issue. > > Bhawesh Choudhary has updated the pull request incrementally with one > additional commit since the last revision: > > Updated test as per review comment Marked as reviewed by kcr (Lead). - PR: https://git.openjdk.java.net/jfx/pull/279
RFR: 8252446: Screen.getScreens() is empty sometimes
As noted in the bug report, we get a pair of change events every time the list of screens changes. First, a change is sent with an empty list of screens and then a change is sent with the new list of screens. This happens whenever a monitor is plugged in or unplugged. It also happens on Mac at application startup. As noted in the bug the reason for this is because the `updateConfiguration` method makes two separate calls on the list of screens, `clear` and `addAll`, rather than calling `setAll`. The latter ensures that only a single change event is delivered. I verified that before this fix, the example program attached to the bug works correctly after the fix. I wrote a unit test. It ends up being skipped on Windows and Linux since we don't get an initial change event. On Mac the test fails without the fix and passes with the fix. - Commit messages: - 8252446: Screen.getScreens() is empty sometimes Changes: https://git.openjdk.java.net/jfx/pull/295/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx&pr=295&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8252446 Stats: 107 lines in 2 files changed: 103 ins; 2 del; 2 mod Patch: https://git.openjdk.java.net/jfx/pull/295.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/295/head:pull/295 PR: https://git.openjdk.java.net/jfx/pull/295
Integrated: 8252387: Deprecate for removal css Selector and ShapeConverter constructors
On Fri, 28 Aug 2020 14:55:55 GMT, Bhawesh Choudhary wrote: > Deprecate the public constructor of javafx.css.Selector as it should not be > public due to only being extended by > classes in same package. Deprecate the public constructor of > javafx.css.converter.ShapeConverter as its a singleton > class. This pull request has now been integrated. Changeset: a5ecfb68 Author:Bhawesh Choudhary Committer: Nir Lisker URL: https://git.openjdk.java.net/jfx/commit/a5ecfb68 Stats: 14 lines in 2 files changed: 0 ins; 14 del; 0 mod 8252387: Deprecate for removal css Selector and ShapeConverter constructors Reviewed-by: nlisker, kcr - PR: https://git.openjdk.java.net/jfx/pull/290
Re: RFR: 8185886: Improve scrolling performance of TableView and TreeTableView [v4]
On Wed, 2 Sep 2020 10:46:48 GMT, Jeanette Winzenburg
wrote:
>> When the startup time is measured by eye, the impression changes depending
>> on the individual difference.
>> The effect of runLater affects your experience.
>>
>> However, I succeeded in further improving performance by eliminating another
>> bottleneck in TreeTableView. Of course, it
>> also includes improvements in startup time.
>> I plan to commit at a later date.
>
>>
>>
>> When the startup time is measured by eye, the impression changes depending
>> on the individual difference.
>
> my eye is a precision instrument :) Seriously, who would do such a thingy?
> Obviously, it must be measured, for zeroth
> approximation doing so crudely by comparing currentMillis before/after
> showing is good enough to "see" the tendency.
>> The effect of runLater affects your experience.
>
> that's why you shouldn't do it when trying to gain insight into timing issues
> ;)
>
>>
>> However, I succeeded in further improving performance by eliminating another
>> bottleneck in TreeTableView. Of course, it
>> also includes improvements in startup time.
>
> cool :)
Column virtualization causes shortness of breath when scrolling a large stroke
horizontally.
This does not happen when stroking on the scrollbar. Looks like a potential
problem with VirtualFlow.
If you are worried about shortness of breath, the following code will help
reduce the problem.
Java
private static final int OVERLAP_MARGIN = 500;
private static boolean isOverlap(double start, double end, double start2,
double end2){
start = Math.max(start-OVERLAP_MARGIN, start2);
end = Math.min(end+OVERLAP_MARGIN, end2);
return (start<=end2 && end >= start2);
}
-
PR: https://git.openjdk.java.net/jfx/pull/125
Re: RFR: 8185886: Improve scrolling performance of TableView and TreeTableView [v4]
> If there are many columns, the current TableView will stall scrolling.
> Resolving this performance issue requires column
> virtualization. Virtualization mode is enabled when the row height is fixed
> by the following method.
> `tableView.setFixedCellSize(height)`
>
> This proposal includes a fix because the current code does not correctly
> implement column virtualization.
>
> The improvement of this proposal can be seen in the following test program.
>
> Java
> import java.util.Arrays;
> import java.util.Collections;
>
> import javafx.animation.AnimationTimer;
> import javafx.application.Application;
> import javafx.beans.property.SimpleStringProperty;
> import javafx.collections.ObservableList;
> import javafx.scene.Scene;
> import javafx.scene.control.Button;
> import javafx.scene.control.TableColumn;
> import javafx.scene.control.TableView;
> import javafx.scene.layout.BorderPane;
> import javafx.scene.layout.HBox;
> import javafx.stage.Stage;
>
> public class BigTableViewTest2 extends Application {
> private static final boolean USE_WIDTH_FIXED_SIZE = false;
> private static final boolean USE_HEIGHT_FIXED_SIZE = true;
> //private static final int COL_COUNT=30;
> //private static final int COL_COUNT=300;
> //private static final int COL_COUNT=600;
> private static final int COL_COUNT = 1000;
> private static final int ROW_COUNT = 1000;
>
> @Override
> public void start(final Stage primaryStage) throws Exception {
> final TableView tableView = new TableView<>();
>
> //tableView.setTableMenuButtonVisible(true); //too heavy
> if (USE_HEIGHT_FIXED_SIZE) {
> tableView.setFixedCellSize(24);
> }
>
> final ObservableList> columns =
> tableView.getColumns();
> for (int i = 0; i < COL_COUNT; i++) {
> final TableColumn column = new
> TableColumn<>("Col" + i);
> final int colIndex = i;
> column.setCellValueFactory((cell) -> new
> SimpleStringProperty(cell.getValue()[colIndex]));
> columns.add(column);
> if (USE_WIDTH_FIXED_SIZE) {
> column.setPrefWidth(60);
> column.setMaxWidth(60);
> column.setMinWidth(60);
> }
> }
>
> final Button load = new Button("load");
> load.setOnAction(e -> {
> final ObservableList items =
> tableView.getItems();
> items.clear();
> for (int i = 0; i < ROW_COUNT; i++) {
> final String[] rec = new String[COL_COUNT];
> for (int j = 0; j < rec.length; j++) {
> rec[j] = i + ":" + j;
> }
> items.add(rec);
> }
> });
>
> final Button reverse = new Button("reverse columns");
> reverse.setOnAction(e -> {
> final TableColumn[] itemsArray =
> columns.toArray(new TableColumn[0]);
> Collections.reverse(Arrays.asList(itemsArray));
> tableView.getColumns().clear();
>
> tableView.getColumns().addAll(Arrays.asList(itemsArray));
> });
>
> final Button hide = new Button("hide % 10");
> hide.setOnAction(e -> {
> for (int i = 0, n = columns.size(); i < n; i++) {
> if (i % 10 == 0) {
> columns.get(i).setVisible(false);
> }
> }
> });
>
> final BorderPane root = new BorderPane(tableView);
> root.setTop(new HBox(8, load, reverse, hide));
>
> final Scene scene = new Scene(root, 800, 800);
> primaryStage.setScene(scene);
> primaryStage.show();
> this.prepareTimeline(scene);
> }
>
> public static void main(final String[] args) {
> Application.launch(args);
> }
>
> private void prepareTimeline(final Scene scene) {
> new AnimationTimer() {
> @Override
> public void handle(final long now) {
> final double fps =
> com.sun.javafx.perf.PerformanceTracker.getSceneTracker(scene).getInstantFPS();
> ((Stage) scene.getWindow()).setTitle("FPS:" +
> (int) fps);
> }
> }.start();
> }
> }
>
> Java
> import javafx.animation.AnimationTimer;
> import javafx.application.Application;
> import javafx.application.Platform;
> import javafx.beans.property.ReadOnlySt
