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

[David Grieve]

On Tue, 12 Nov 2019 16:55:45 GMT, Ajit Ghaisas <aghaisas at openjdk.org> wrote:

> On Tue, 12 Nov 2019 16:52:54 GMT, Ajit Ghaisas <aghaisas at openjdk.org> wrote:
> 
>> On Tue, 12 Nov 2019 16:45:04 GMT, Ajit Ghaisas <aghaisas at openjdk.org> wrote:
>> 
>>> **Issue :**
>>> https://bugs.openjdk.java.net/browse/JDK-8193445
>>> 
>>> **Background :**
>>> The CSS performance improvement done in [JDK-8151756](https://bugs.openjdk.java.net/browse/JDK-8151756) had to be backed out due to functional regressions reported in [JDK-8185709](https://bugs.openjdk.java.net/browse/JDK-8185709), [JDK-8183100](https://bugs.openjdk.java.net/browse/JDK-8183100) and [JDK-8168951](https://bugs.openjdk.java.net/browse/JDK-8168951).
>>> Refer to [JDK-8183100](https://bugs.openjdk.java.net/browse/JDK-8183100) for more details on this backout. 
>>> 
>>> **Description :**
>>> This PR reintroduces the CSS performance improvement fix done in [JDK-8151756](https://bugs.openjdk.java.net/browse/JDK-8151756) while addressing the functional regressions that were reported in [JDK-8185709](https://bugs.openjdk.java.net/browse/JDK-8185709), [JDK-8183100](https://bugs.openjdk.java.net/browse/JDK-8183100) and [JDK-8168951](https://bugs.openjdk.java.net/browse/JDK-8168951).
>>> For ease of review, I have made two separate commits -
>>> 1) [Commit 1](https://github.com/openjdk/jfx/pull/34/commits/d964675ebc2a42f2fd6928b773819502683f2334) - Reintroduces the CSS performance improvement fix done in [JDK-8151756](https://bugs.openjdk.java.net/browse/JDK-8151756) - most of the patch applied cleanly.
>>> 2) [Commit 2 ](https://github.com/openjdk/jfx/pull/34/commits/12ea8220a730ff8d98d520ce870691d54f0de00f)- fixes the functional regressions keeping performance improvement intact + adds a system test
>>> 
>>> **Root Cause :**
>>> CSS performance improvement fix proposed in [JDK-8151756 ](https://bugs.openjdk.java.net/browse/JDK-8151756)correctly avoids the redundant CSS reapplication to children of a Parent. 
>>> What was missed earlier in [JDK-8151756 ](https://bugs.openjdk.java.net/browse/JDK-8151756) fix : "CSS reapplication to the Parent itself”. 
>>> This missing piece was the root cause of all functional regressions reported against [JDK-8151756](https://bugs.openjdk.java.net/browse/JDK-8151756)
>>> 
>>> **Fix :**
>>> Fixed the identified root cause. See commit 2.
>>> 
>>> **Testing :**
>>> 1. All passing unit tests continue to pass
>>> 2. New system test (based on [JDK-8209830](https://bugs.openjdk.java.net/browse/JDK-8209830)) added in this PR - fails before this fix and passes after the fix
>>> 3. System test JDK8183100Test continues to pass
>>> 4. All test cases attached to regressions [JDK-8185709](https://bugs.openjdk.java.net/browse/JDK-8185709), [JDK-8183100](https://bugs.openjdk.java.net/browse/JDK-8183100) and [JDK-8168951](https://bugs.openjdk.java.net/browse/JDK-8168951) pass with this fix
>>> 
>>> In addition, testing by community with specific CSS performance / functionality will be helpful.
>>> 
>>> ----------------
>>> 
>>> Commits:
>>>  - 12ea8220: Fix for functional regressions of JDK-8151756 + add a sytem test
>>>  - d964675e: Reintroduce JDK-8151756 CSS performance fix
>>> 
>>> Changes: https://git.openjdk.java.net/jfx/pull/34/files
>>>  Webrev: https://webrevs.openjdk.java.net/jfx/34/webrev.00
>>>   Issue: https://bugs.openjdk.java.net/browse/JDK-8193445
>>>   Stats: 121 lines in 5 files changed: 104 ins; 0 del; 17 mod
>>>   Patch: https://git.openjdk.java.net/jfx/pull/34.diff
>>>   Fetch: git fetch https://git.openjdk.java.net/jfx pull/34/head:pull/34
>> 
>> Reviewers: @kevinrushforth and @arapte
> 
> It will be helpful if I can get some feedback from community with additional testing apart from what is mentioned in the description.

Has this been checked with SubScene? Two cases would be 1) SubScene inheriting a style from its parent, and 2) behavior of a parent in the SubScene itself. I don't expect that this would be an issue, but there is some special handling of CSS in SubScene IIRC.

PR: https://git.openjdk.java.net/jfx/pull/34