JDK-8193445 Performance Test

[Kevin Rushforth]

My thoughts as well. I would love to see a safe robust fix for this 
issue, but there is a very real possibility of a regression: We thought 
we had a safe fix earlier and only later discovered the (multiple) 
regressions a few months after it was released.

One possible way to improve performance is to allow applications to 
manage when it is safe to skip applying CSS to certain nodes under app 
control, as proposed in 
https://bugs.openjdk.java.net/browse/JDK-8173301. I'm not sure that 
would be as effective, but at least it would only affect apps that "opt 
in" and only for those parts of the scene graph that are so designated.

I do like the idea of coming up with better tests that can be used to 
validate any potential future fix.

-- Kevin


On 11/7/2018 5:37 AM, David Grieve wrote:
> 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;
>>          }
>>      }
>> }
>