On Mon, 13 Jul 2026 20:30:07 GMT, Marius Hanl <[email protected]> wrote:
> Since the last [PR](https://github.com/openjdk/jfx/pull/2201) made the parent > `styleHelper` creation more eager, creating and adding many nodes will be > slower (initially). > > The fix is actually something I already had in mind when I did the previous > PR: Find the ancestor lazily. > Now, when we create a `CssStyleHelper` we will not yet set the > `firstStyleableAncestor`. > This will improve the performance to the level before while all tests still > pass. > > This makes sense because `CssStyleHelper` is created (top-down) but not > immediately used. It can also be (re)created when a `Node` changes its > `Parent` (or deferred from there). > > Later, `transitionToState` is called (top-down) which will then need the > `firstStyleableAncestor` (and this is also where we will compute it now). > Since in the meantime the scene structure could change, it makes sense to do > that as late as possible. > > The days before I also investigated > [JDK-8187955](https://bugs.openjdk.org/browse/JDK-8187955) and found a good > improvement as well (no recursion needed, we can again use the preexisting > loop). Since we now create the `firstStyleableAncestor` lazy, it makes sense > to combine it, as otherwise the `isUserSetFont` need to be changed which I > think is more risky. > > Also did some digging about `isUserSetFont`, and was added/changed in this > tickets: > - `isUserSetFont` code added: https://bugs.openjdk.org/browse/JDK-8123731 > - `isUserSetFont` code changed: https://bugs.openjdk.org/browse/JDK-8093577 > > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). I think this is a good direction, but I worry about the risk of functional regression. CSS is, as you know, notoriously prone to regressions. It's difficult to test and there are a lot of paths to consider. And any time we do lazy evaluation, it needs to be tested very carefully. One concrete concern: The lazy eval approach will change when `CssStyleHelper parentHelper` is created. That could affect the validity of the cache key. Given that we are T-47 hours to the JavaFX 27 RDP1 fork, I think we are better off backing out JDK-8268657 for 27, and then redoing it, along with _this_ fix in JavaFX 28 when we we will have the time to do more evaluation and testing to catch regressions before they happen, and more time to deal with any regressions that might be missed. What do other reviewers think? ------------- PR Comment: https://git.openjdk.org/jfx/pull/2210#issuecomment-4971866495
