8259046: ViewPainter.ROOT_PATHS holds reference to Scene causing memory leak

[kevinrushforth]

Prism implements a dirty region optimization, where in many cases, only part of the scene graph is re-rendered when something changes. In support of this, the ViewPainter class in the Quantum Toolkit keeps an array of node paths, ROOT_PATHS, which is a list of sub-trees in the scene graph that need to be rendered based on a change to that node. The entries in the ROOT_PATHS array are intended to be transient during the rendering of a single pass of a single scene. They are recreated every time a scene is rendered. The leak occurs because the entries are not cleared after being used. The fix is to clear each entry after it is rendered. In addition to static analysis, which shows that the entries are never used again after a frame is rendered, I have done a full build and test, including manual tests, to be sure that there is no regression.

I have added a test that will fail consistently on Mac and Windows (and intermittently on Linux) without the fix. It passes consistently on all platforms with the fix.

Even though this is a simple change, it is in an area that has historically been fragile, so I would like two reviewers.

/reviewers 2

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8259046: ViewPainter.ROOT_PATHS holds reference to Scene causing memory leak

Reviewers

• Pankaj Bansal (@pankaj-bansal - Committer)
• Ambarish Rapte (@arapte - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jfx pull/388/head:pull/388
$ git checkout pull/388

[bridgekeeper]

👋 Welcome back kcr! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@kevinrushforth
The number of required reviews for this PR is now set to 2 (with at least 1 of role reviewers).

[mlbridge]

Webrevs

• 00: Full (cb3d43a)

[pankaj-bansal]

I did ran full test and did some run some manual tests also. The changes look fine to me.

[arapte]

Looks good to me.

[openjdk]

@kevinrushforth This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8259046: ViewPainter.ROOT_PATHS holds reference to Scene causing memory leak

Reviewed-by: pbansal, arapte

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 10 new commits pushed to the master branch:

• d4058a1: 8260475: Deprecate for removal protected access members in DateTimeStringConverter
• 887a443: Merge
• 0f20a98: 8252389: Fix mistakes in FX API docs
• 425c335: 8254836: Cherry pick GTK WebKit 2.30.3 changes
• 215384d: 8260246: Ensemble: Update version of Lucene to 7.7.3
• e481f8c: 8260163: IrresponsiveScriptTest.testInfiniteLoopInScript unit test fails on Windows
• 217a8cb: 8259680: Need API to query states of CAPS LOCK and NUM LOCK keys
• db6941d: 8256283: IndexOutOfBoundsException when sorting a TreeTableView
• b0a404d: 8259635: Update to 610.2 version of WebKit
• 6c1a0ca: 8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[kevinrushforth]

/integrate

[openjdk]

@kevinrushforth Since your change was applied there have been 10 commits pushed to the master branch:

• d4058a1: 8260475: Deprecate for removal protected access members in DateTimeStringConverter
• 887a443: Merge
• 0f20a98: 8252389: Fix mistakes in FX API docs
• 425c335: 8254836: Cherry pick GTK WebKit 2.30.3 changes
• 215384d: 8260246: Ensemble: Update version of Lucene to 7.7.3
• e481f8c: 8260163: IrresponsiveScriptTest.testInfiniteLoopInScript unit test fails on Windows
• 217a8cb: 8259680: Need API to query states of CAPS LOCK and NUM LOCK keys
• db6941d: 8256283: IndexOutOfBoundsException when sorting a TreeTableView
• b0a404d: 8259635: Update to 610.2 version of WebKit
• 6c1a0ca: 8228363: ContextMenu.show with side=TOP does not work the first time in the presence of CSS

Your commit was automatically rebased without conflicts.

Pushed as commit 676cf3e.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.