8193445: JavaFX CSS is applied redundantly leading to significant performance degradation

modules/javafx.graphics/src/main/java/javafx/scene/Node.java

@@ -1051,13 +1051,15 @@ public void fireSuperValueChangedEvent() {
         }
     }
 
-    private void invalidatedScenes(Scene oldScene, SubScene oldSubScene) {
+    // reapplyCSS should be true for root elements when they are added, and is false for children
+    // of the root element. This prevents CSS being reapplied recursively, as noted in JDK-8151756.
+    private void invalidatedScenes(Scene oldScene, SubScene oldSubScene, boolean reapplyCSS) {
         Scene newScene = sceneProperty().get();
         boolean sceneChanged = oldScene != newScene;
         SubScene newSubScene = subScene;
 
         if (getClip() != null) {
-            getClip().setScenes(newScene, newSubScene);
+            getClip().setScenes(newScene, newSubScene, reapplyCSS);
         }
         if (sceneChanged) {
             updateCanReceiveFocus();
@@ -1091,7 +1093,9 @@ private void invalidatedScenes(Scene oldScene, SubScene oldSubScene) {
         }
         updateTreeShowing();
 
-        if (sceneChanged) reapplyCSS();
+        if (sceneChanged && reapplyCSS) {
+            reapplyCSS();
+        }
 
         if (sceneChanged && !isDirtyEmpty()) {
             //Note: no need to remove from scene's dirty list
@@ -1150,16 +1154,16 @@ private void invalidatedScenes(Scene oldScene, SubScene oldSubScene) {
         }
     }
 
-    final void setScenes(Scene newScene, SubScene newSubScene) {
+    final void setScenes(Scene newScene, SubScene newSubScene, boolean reapplyCSS) {
         Scene oldScene = sceneProperty().get();
         if (newScene != oldScene || newSubScene != subScene) {
             scene.set(newScene);
             SubScene oldSubScene = subScene;
             subScene = newSubScene;
-            invalidatedScenes(oldScene, oldSubScene);
+            invalidatedScenes(oldScene, oldSubScene, reapplyCSS);
             if (this instanceof SubScene) { // TODO: find better solution
                 SubScene thisSubScene = (SubScene)this;
-                thisSubScene.getRoot().setScenes(newScene, thisSubScene);
+                thisSubScene.getRoot().setScenes(newScene, thisSubScene, reapplyCSS);
             }
         }
     }
@@ -1180,8 +1184,10 @@ public final ReadOnlyObjectProperty<Scene> sceneProperty() {
      * Exists for Parent and LightBase
      */
     void scenesChanged(final Scene newScene, final SubScene newSubScene,
-                       final Scene oldScene, final SubScene oldSubScene) { }
-
+                       final Scene oldScene, final SubScene oldSubScene) {
+        // On scenes change, reapply CSS for this Node
+        reapplyCSS();
+    }
 
     /**
      * The id of this {@code Node}. This simple string identifier is useful for
@@ -6976,13 +6982,13 @@ protected void invalidated() {
                         } else {
                             if (oldClip != null) {
                                 oldClip.clipParent = null;
-                                oldClip.setScenes(null, null);
+                                oldClip.setScenes(null, null, /* reapplyCSS */ false);
                                 oldClip.updateTreeVisible(false);
                             }
 
                             if (newClip != null) {
                                 newClip.clipParent = Node.this;
-                                newClip.setScenes(getScene(), getSubScene());
+                                newClip.setScenes(getScene(), getSubScene(), /* reapplyCSS */ false);
                                 newClip.updateTreeVisible(true);
                             }
 

modules/javafx.graphics/src/main/java/javafx/scene/Parent.java

@@ -368,7 +368,7 @@ protected void onChanged(Change<Node> c) {
                             relayout = true;
                         }
                         node.setParent(Parent.this);
-                        node.setScenes(getScene(), getSubScene());
+                        node.setScenes(getScene(), getSubScene(), /* reapplyCSS */ true);
                         // assert !node.boundsChanged;
                         if (node.isVisible()) {
                             geomChanged = true;
@@ -601,7 +601,7 @@ protected void onProposedChange(final List<Node> newNodes, int[] toBeRemoved) {
                     }
                     if (old.getParent() == Parent.this) {
                         old.setParent(null);
-                        old.setScenes(null, null);
+                        old.setScenes(null, null, /* reapplyCSS */ false);
                     }
                     // Do not add node with null scene to the removed list.
                     // It will not be processed in the list and its memory
@@ -756,6 +756,7 @@ final void toBack(Node node) {
     @Override
     void scenesChanged(final Scene newScene, final SubScene newSubScene,
                        final Scene oldScene, final SubScene oldSubScene) {
+        super.scenesChanged(newScene, newSubScene, oldScene, oldSubScene);
 
         if (oldScene != null && newScene == null) {
             // RT-34863 - clean up CSS cache when Parent is removed from scene-graph
@@ -768,7 +769,7 @@ void scenesChanged(final Scene newScene, final SubScene newSubScene,
         }
 
         for (int i=0; i<children.size(); i++) {
-            children.get(i).setScenes(newScene, newSubScene);
+            children.get(i).setScenes(newScene, newSubScene, /* reapplyCSS */ false);
         }
 
         final boolean awaitingLayout = layoutFlag != LayoutFlags.CLEAN;
@@ -787,6 +788,7 @@ void scenesChanged(final Scene newScene, final SubScene newSubScene,
                 }
             }
         }
+
     }
 
     @Override

modules/javafx.graphics/src/main/java/javafx/scene/Scene.java

@@ -1218,11 +1218,11 @@ protected void invalidated() {
                     }
 
                     if (oldRoot != null) {
-                        oldRoot.setScenes(null, null);
+                        oldRoot.setScenes(null, null, /* reapplyCSS */ false);
                     }
                     oldRoot = _value;
                     _value.getStyleClass().add(0, "root");
-                    _value.setScenes(Scene.this, null);
+                    _value.setScenes(Scene.this, null, /* reapplyCSS */ true);
                     markDirty(DirtyBits.ROOT_DIRTY);
                     _value.resize(getWidth(), getHeight()); // maybe no-op if root is not resizable
                     _value.requestLayout();

modules/javafx.graphics/src/main/java/javafx/scene/SubScene.java

@@ -313,11 +313,11 @@ protected void invalidated() {
 
                     if (oldRoot != null) {
                         StyleManager.getInstance().forget(SubScene.this);
-                        oldRoot.setScenes(null, null);
+                        oldRoot.setScenes(null, null, /* reapplyCSS */ false);
                     }
                     oldRoot = _value;
                     _value.getStyleClass().add(0, "root");
-                    _value.setScenes(getScene(), SubScene.this);
+                    _value.setScenes(getScene(), SubScene.this, /* reapplyCSS */ true);
                     markDirty(SubSceneDirtyBits.ROOT_SG_DIRTY);
                     _value.resize(getWidth(), getHeight()); // maybe no-op if root is not resizable
                     _value.requestLayout();

tests/system/src/test/java/test/robot/javafx/scene/CSSPerf_JDK8193445Test.java

@@ -0,0 +1,96 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package test.robot.javafx.scene;
+
+import javafx.geometry.Insets;
+import javafx.scene.Scene;
+import javafx.scene.layout.HBox;
+import javafx.scene.layout.BorderPane;
+import javafx.scene.text.Text;
+import javafx.stage.Stage;
+
+import org.junit.Test;
+import test.robot.testharness.VisualTestBase;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * This test is based on the test case reported in JDK-8209830
+ *
+ * Redundant CSS Re-application was avoided in JDK-8193445.
+ * It results in faster application of CSS on Controls (Nodes). In turn,
+ * resulting in improved Node creation/addition time to a Scene.
+ *
+ * The goal of this test is *NOT* to measure absolute performance, but to show
+ * creating and adding 300 Nodes to a scene does not take more than a
+ * particular threshold of time.
+ *
+ * The selected thresold is larger than actual observed time.
+ * It is not a benchmark value. It is good enough to catch the regression
+ * in performance, if any.
+ */
+
+public class CSSPerf_JDK8193445Test extends VisualTestBase {
+
+    private Stage testStage;
+    private Scene testScene;
+    private BorderPane pane = new BorderPane();
+    private long mSec = 0;
+
+    @Test(timeout = 15000)
+    public void testTimeForAdding300NodesToScene() {
+        runAndWait(() -> {
+            testStage = getStage();
+            testScene = new Scene(pane);
+            testStage.setScene(testScene);
+            testStage.show();
+        });
+
+        waitFirstFrame();
+
+        // Measure time to create and add 300 Nodes to Scene
+        runAndWait(() -> {
+            long startTime = System.currentTimeMillis();
+
+            HBox hbox = new HBox();
+            for (int i = 0; i < 300; i++) {
+                hbox = new HBox(new Text("y"), hbox);
+                final HBox h = hbox;
+                h.setPadding(new Insets(1));
+            }
+            pane.setCenter(hbox);
+
+            long endTime = System.currentTimeMillis();
+
+            mSec = endTime - startTime;
+        });
+
+        System.out.println("Time to create and add 300 nodes to a Scene = " + mSec + " mSec");
+
+        // NOTE : 400 mSec is not a benchmark value
+        // It is good enough to catch the regression in performance, if any
+        assertTrue("Time to add 300 Nodes is more than 400 mSec", mSec < 400);
+    }
+}