JDK-8151756 : JavaFX CSS is applied redundantly leading to significant performance degradation
  • Type: Bug
  • Component: javafx
  • Sub-Component: controls
  • Affected Version: 8u72
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2016-03-12
  • Updated: 2020-01-31
  • Resolved: 2016-03-16
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 JDK 9
8u102Fixed 9Fixed
Related Reports
Relates :  
JDK-8168951 - Colors not display on LineChart
Relates :  
JDK-8193445 - JavaFX CSS is applied redundantly leading to significant performance degradation
Relates :  
JDK-8183100 - Styles not applied reliably after Java 8u92
Relates :  
JDK-8145885 - StackOverflow in rendering if two many level of pane (Hbox, VBox, ...)
Description
When a Node hierarchy is added to the Scene CSS is applied multiple times depending on the depth of the node in the added hierarchy.  For every level of depth in the hierarchy the CSS is applied an extra time.

See attached test case where a VBox containing another VBox containing a Rectangle is added to the Scene.  Run the application in a debugger. When the UI appears set a breakpoint at Node.reapplyCss() then hit the button.  Note that the breakpoint is hit three times for the Rectangle, twice for the 'parent2' VBox, and once for the 'parent1' VBox.

This leads to significantly poorer performance and causes far more garbage to be generated than it should.
Comments
Note that since this fix has been backed out, a new bug has been filed to track this performance issue: JDK-8193445.
07-11-2018
This has resulted in regressions reported in JDK-8185709, JDK-8183100, JDK-8168951. I have a fix for above regressions : http://cr.openjdk.java.net/~aghaisas/fx/8183100/webrev.0/ I need help in assessing impact of my fix on performance gained in JDK-8151756.
28-11-2017
I don't have a reproduceable small snippet but this changes most likely broke my application who uses getUserAgentStylesheet(). In 8u92 things worked perfectly, starting with 8u102 the getUserAgengtStylesheet() only gets applied when the node is added to the SG the first time. As this is the only change who might have an effect on CSS application I guess this one broke me. I tried to reproduce in a small test case but unfortunately I could reproduce this in a stripped down version. The only work-around so far I found was invoke impl_reapplyCSS() in a runLater call
06-02-2017
Backported to 8u-dev. Changeset: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/a1a7ab147c1b
02-04-2016
Approved to backport to 8u-dev (8u102).
01-04-2016
This patch applies cleanly to 8u-dev. I have run tests against 8u-dev and cannot see any regressions. All unit tests continue to pass, and the controls sanity tests continue to pass. I think this patch can now be backported to 8u-dev.
30-03-2016
Changeset: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/168be8cac9e6 Will not backport immediately - will give this a few weeks to be tested in 9dev, and if everything goes smoothly, we can backport then. Scott - if this gets forgotten about, please chase me up.
16-03-2016
Thanks for testing. Let me know if you see anything unexpected regarding calls to reapplyCSS().
15-03-2016
I applied your patch to 8u-dev and saw a 33% improvement in my application on OS X. I'm not set up to build JavaFX on Windows so I was unable to test there. I'm also seeing different output from the pulse logger on OS X than I get on Windows, so I can't quite isolate the timing of the CSS pass. So far, no undesirable side-effects.
15-03-2016
I run some performance and regression tests and can not see any issues with this patch.
15-03-2016
Thanks for reviewing David. I'll do a bunch more testing, and will look into pushing this in a few weeks time, once we are past the jigsaw merge window. Given the scale of this patch, it is likely that this could be backported, but again it will require substantial testing first.
15-03-2016
I think this is good. The only thing I'm not sure about passing false for the clipping node. I don't recall if CSS is applied to the clip node.
15-03-2016
Attaching one possible patch that appears to resolve this issue. I haven't tested exhaustively however, so it is possible something may not be accurate. David, if you have a spare moment, I'd appreciate your thoughts on this approach. Basically, I've changed things so that the reapplyCSS() call is only on the root node, when a scene change occurs. All children do not get to call reapplyCSS() themselves (but of course, they should be called from the root node in any case, and this happens after all nodes get their new Scene applied).
14-03-2016
I turned on pulse logging just to get some more information. (Profiling with jvisualvm doesn't seem to be working properly.) This is what I get when I trigger this condition in my application: PULSE: 2578 [423ms:3362ms] T35 (-423 +423ms): Layout Pass T35 (0 +0ms): CSS Pass T35 (0 +0ms): Layout Pass T35 (0 +2725ms): CSS Pass T35 (2725 +402ms): Layout Pass T35 (3128 +25ms): Update bounds T35 (3153 +0ms): Waiting for previous rendering T35 (3153 +79ms): Copy state to render graph T16 (3233 +0ms): Dirty Opts Computed T16 : 4 different dirty regions to render ... (snipped several "T16 : Slow background path for null") Counters: CacheFilter rebuilding: 1 Cached region background image used: 2026 Cached region shape image used: 22 NGRegion renderBackgroundShape slow path: 3 NGRegion renderBackgrounds slow path: 42 Nodes rendered: 10515 Nodes visited during render: 10636 Rendering region background image to cache: 3 Rendering region shape image to cache: 1 As you can see the pulse is spending most of the time in the CSS Pass, which is why I filed this issue when I discovered the redundant processing.
14-03-2016
We have quarterly 8u release, so #3 is taken care of. Once a fix is found (#1) the real question will be how safe it is (#2). I'll add the "8-bp" label to indicate that it is a candidate for a backport.
14-03-2016
Scott, that really depends on (at least) three things: 1) When / if I can find a fix (hoping David can give some pointers here). 2) The complexity of the fix (the simpler it is, the more likely it can be backported). 3) The availability of an 8 update release that can accept it (I honestly don't keep track of the 8u releases, so I have no idea what is coming up). It would be my intention to try to backport this kind of fix however, so that is the default starting point.
13-03-2016
When a proper fix is found is there decent chance of getting it back ported to 8u?
13-03-2016
In my analysis this morning I can see that the code has the following setup: a Rectangle node is inserted inside a VBox (parent2), and that VBox is inside another VBox (parent1). This outer VBox is not in the scenegraph until a button is clicked. When the button is clicked it is added, and at this point CSS is applied a number of times, all for the same reason: the scene has changed (from null to a valid scene). The order of operation is the following: 1) Rectangle: CSSFlags.CLEAN 2) VBox (parent2): CSSFlags.DIRTY_BRANCH 3) Rectangle: CSSFlags.CLEAN 4) VBox (parent1): CSSFlags.DIRTY_BRANCH 5) VBox (parent2): CSSFlags.UPDATE 6) Rectangle: CSSFlags.CLEAN I know it isn't the right solution, but simply commenting out the reapplyCss() call when scene is changed works fine :-) I'll keep looking into this and will try to find a solution.
13-03-2016