On Tue, 30 Jun 2026 12:04:48 GMT, Marius Hanl <[email protected]> wrote:
>> This PR is an optimization for `Node.styleClass` and `Parent.stylesheets`. >> Instead of always initializing both properties with an empty list, we are >> creating (therefore allocating) the list on the first access instead. >> Similar to many other lazy properties. >> >> Why? >> - `Parent.getStylesheets()` is very rarely used by developers and JavaFX >> code. So this list is very often completely unused and empty. Most >> developers usually only add stylesheets at the `Scene` >> - `Node.getStyleClass()` is usually not used (empty) for layout containers >> such as `Pane` or `Group` or shapes. A JavaFX App usually consists of a good >> amount of such containers (or shapes) >> >> So that our CSS code is not initializing both lists on access, I added >> related `NodeHelper` and `ParentHelper` methods to return `null` when both >> lists were not initialized and therefore used. Otherwise we return the list >> as before. >> This will save us some memory and allocation, which is both good for the >> memory consumption but also for `Node` / `Parent` creation (time). >> >> Added documentation and tests. Will do some measurements with some apps very >> soon and attach it here. >> >> --- >> >> # Benchmarks >> >> I wrote a very small scene graph analyzer snippet to measure the memory gain. >> Feel free to test this on your own apps! >> 1. Get the `SceneGraphAnalyzer` here: >> [SceneGraphAnalyzer](https://gist.github.com/Maran23/38beca5b043e547e1a84749e3162c0b2) >> 2. Add this code to your `Scene` and press the shortcut `F12` when all of >> the UI is loaded: >> >> scene.setOnKeyPressed(event -> { >> if (event.getCode() == KeyCode.F12) { >> var analyzer = new SceneGraphAnalyzer(); >> var res = analyzer.analyze(scene); >> res.print(); >> } >> }); >> >> 3. If you have a modular app, add the following VM argument: `--add-opens >> javafx.graphics/javafx.scene=yourapp` >> >> ## Projects >> >> 1. Tested with >> [JFXCentral](https://github.com/dlsc-software-consulting-gmbh/jfxcentral2) >> >> >> ╔══════════════════════════════════════════════════╗ >> ║ Scene Graph Analysis ║ >> ╠══════════════════════════════════════════════════╣ >> ║ Total nodes: 654 ║ >> ║ ├─ Parent nodes: 388 ║ >> ║ └─ Leaf nodes: 266 ║ >> ╠══════════════════════════════════════════════════╣ >> ║ Null styleClass: 19 / 654 ( 2.9%) ║ >> ║ Null stylesheets: 3... > > Marius Hanl has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains four additional > commits since the last revision: > > - Merge branch 'master' of https://github.com/openjdk/jfx into > 8386663-Stylesheet/StyleClass-list-should-be-lazily-initialized > - MenuItem toString() should also use the new styleClass way. > > TwoLevelFocusListBehavior was very weirdly relying on the toString() > behavior > - Move styleClass.toString into Node.toString() > - 8386663: Stylesheet/StyleClass list should be lazily initialized Please document the toString() format change in the JBS ticket and this PR description. I don't know whether a CSR is needed, since the actual format has not been documented, but the fact that multiple tests fail with this change indicate that the applications that rely on the exact string (and not on `.contains()`) will break. modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java line 220: > 218: @Test > 219: public void testCssProcessingDoesNotInitializeStyleClass() { > 220: Rectangle node = new Rectangle(); the `node` is never attached to the scene graph, so the pulses do nothing, and also css processing is not happening, right? modules/javafx.graphics/src/test/java/test/javafx/scene/ParentTest.java line 113: > 111: assertNull(stylesheets); > 112: > 113: Group g = new Group(); The `node` is never attached to the `Group` or the scene graph, same issue. ------------- PR Review: https://git.openjdk.org/jfx/pull/2191#pullrequestreview-4603427435 PR Review Comment: https://git.openjdk.org/jfx/pull/2191#discussion_r3501287219 PR Review Comment: https://git.openjdk.org/jfx/pull/2191#discussion_r3501293758
