On Thu, 18 Jun 2026 16:34:19 GMT, Andy Goryachev <[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...
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/css/SimpleSelector.java 
> line 350:
> 
>> 348:          */
>> 349: 
>> 350:         if (styleClassNames == null || styleClassNames.size() < 
>> selectorStyleClassNames.size()) {
> 
> would it make more sense, instead of adding the null check here, move it up 
> to after L278, to avoid unnecessary check?

I think this will not really matter. 
Just made the change locally to check what looks/feels better and personally it 
felt better in `matchesStyleClasses`.

Because it bascially reads like "well, a null `styleClass` will not match 
anything to just return". But just my personally feeling here really.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/2191#discussion_r3446315276

Reply via email to