On Wed, 8 Jan 2025 17:39:10 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> modules/javafx.graphics/src/main/java/javafx/scene/Parent.java line 479: >> >>> 477: >>> n.pseudoClassStateChanged(NTH_EVEN_CHILD_PSEUDO_CLASS, i % 2 != 0); >>> 478: >>> n.pseudoClassStateChanged(NTH_ODD_CHILD_PSEUDO_CLASS, i % 2 == 0); >>> 479: } >> >> With this new code, Nodes with many children being manipulated may incur >> some significant new costs now that these pseudo classes need to be changed >> for potentially many/all children. Where before adding a new first child >> mostly had a cost in shifting the child array with a copy, now we also >> potentially have two listener call backs per child (and I'm unsure, but >> these may even trigger some CSS recalculations or CSS cache invalidations). >> Nodes with many children may suffer from this, and mostly needlessly as >> these pseudo classes will see very limited use. >> >> I think there's some potential for performance regression here that is >> perhaps sufficient to warrant the exclusion of this feature or making it >> optional. > > I think this concern is out of proportion. Generally, nodes will not have > such a huge number of children that simply flipping a few pseudo-classes will > be prohibitively expensive. CSS recalculation will, in most cases, only > happen once per pulse anyway. > > I don't know why we would be optimizing for pathological applications that > register two pseudo-class listeners for each of their tens of thousands of > children in a container, _and_ do heavy work in those listeners. Perhaps it is not as dire, but it may be worth a short check how this works with say 1000 nodes with and without this new code (no need to register listeners, I agree that that is a different problem, I was more worried about how CSS handles this many changes). Perhaps also a case where there is also some style change going on linked to the odd or even pseudo class (although if that is much slower, then I guess that's what the user wanted...) ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1652#discussion_r1907919550
