JDK-8193445 Performance Test

[David Grieve]

One of the dangers of mucking around with the CSS code is whether or not 
the changes break things like popups, dialogs, menus. And whether or not 
the change breaks inline styles versus attributes set in code, versus 
stylesheets added to the scene/subscene/control, versus default 
stylesheets. And whether or not the changes breaks skins for controls.

Reapplying CSS to a Node but not its children could cause a problem if 
there are styles in the parent or the parent's parents that affect the 
children. It seems like bypassing children in reapplyCSS is bound to 
cause a regression.

Also, because a new scene root could affect styles, CSS is reapplied 
after calling sceneChanged - your change puts it before, so I question 
whether or not this will cause a regression. I think if you skip 
reapplying CSS when a Node's scene has changed, then the children won't 
get the correct styles, and this will affect layout.

I haven't spent much time evaluating this change in detail, but I'm 
doubtful that it won't cause regressions.


On 11/7/18 2:57 AM, Dean Wookey wrote:
> Hi,
>
> I was going to ask if it was possible to reopen JDK-8151756 (
> https://bugs.openjdk.java.net/browse/JDK-8151756) since it was fixed but
> reverted in JDK-8183100 (https://bugs.openjdk.java.net/browse/JDK-8183100)
> after causing several regressions. I only noticed now that a followup bug
> was created: JDK-8193445 which reopens the issue.
>
> The code below demonstrates the problem where adding nodes to the scene
> graph all at once performs exponentially slower than adding them one by
> one. I get the following results with jfx11.0.1:
>
> One by one: 138ms
> All at once: 16704ms
>
> I've made a potential fix, different to the one tried previously which
> applies the css as if the nodes were being added one by one:
> https://github.com/DeanWookey/openjdk-jfx/commit/65a1ed82bce262294f1969e9a12e1126ec8a1ec6
>
> It passes the main tests, as well as the systemTest JDK8183100Test.java
> from https://bugs.openjdk.java.net/browse/JDK-8193494.
>
> This is probably not a suitable issue to work on for a first time
> contributor, but perhaps I could work on a performance test if someone can
> point me in the direction of existing performance tests?
>
> Dean
>
> package applycsstest;
>
> import javafx.application.Application;
> import javafx.application.Platform;
> import javafx.scene.Scene;
> import javafx.scene.layout.StackPane;
> import javafx.stage.Stage;
>
> /**
>   *
>   * @author Dean
>   */
> public class ApplyCssTest extends Application {
>
>      @Override
>      public void start(Stage primaryStage) {
>          System.out.println("One by one: " + addToSceneOneByOne() + "ms");
>          System.out.println("All at once: " + addToSceneAllAtOnce() + "ms");
>          Platform.exit();
>      }
>
>      public static void main(String[] args) {
>          launch(args);
>      }
>
>      public long addToSceneOneByOne() {
>          StackPane root = new StackPane();
>          Scene scene = new Scene(root, 300, 250);
>
>          long start = System.currentTimeMillis();
>          StackPane firstChild = new StackPane();
>          root.getChildren().add(firstChild); //add to the scene graph first
>          addNodesOneByOne(1000, firstChild); //then add children one by one
>          return System.currentTimeMillis() - start;
>      }
>
>      public long addToSceneAllAtOnce() {
>          StackPane root = new StackPane();
>          Scene scene = new Scene(root, 300, 250);
>
>          long start = System.currentTimeMillis();
>          StackPane firstChild = new StackPane();
>          addNodesOneByOne(1000, firstChild); //build the node up, then
>          root.getChildren().add(firstChild); //add to scene graph all at once
>          return System.currentTimeMillis() - start;
>      }
>
>      public void addNodesOneByOne(int numToAdd, StackPane parent) {
>          StackPane last = parent;
>          for (int i = 0; i < numToAdd; i++) {
>              StackPane curr = new StackPane();
>              last.getChildren().add(curr);
>              last = curr;
>          }
>      }
> }