CSS Performance regression inTableColumnHeader.resizeColumnToFitContent

[John Hendrikx]

On 01/02/2024 13:00, Robert Lichtenberger wrote:
> Hi,
>
> We are seeing degraded performance in our production application 
> concerning the "fit to content" function of TableViews.
>
> I've developed a little benchmark program that can be found at 
> https://gist.github.com/effad/9eebee0c1e86a8e605cb55ced9485dd4
>
> Here's the last lines of data from runs against different JavaFX 
> versions:
>
> JFX 17.0.10+2 average run time: 848
> JFX 18.0.1+2 average run time: 839
> JFX 19.0.2+1 average run time: 1113
> JFX 20.0.2+3 average run time: 1656
> JFX 21.0.2+5 average run time: 2460
>
> 17 and 18 are almost the same.
>
> The performance penalty of 19 most likely is due to (my own) PR in 
> https://github.com/openjdk/jfx/pull/757, which put's the cell used to 
> measure the needed width of the column into a row.
>
> The penalties from 19 to 20 and 20 to 21 however are worse than that.
>
>
> To investigate further I let the benchmark run (with no warmup, just 3 
> iterations and only 5000 rows) in the profiler Visual VM 2.1.7. 
> Looking at _the_ hotspot of the execution I can see that 
> javafx.scene.CssStyleHelper.transitionToState is consuming most of the 
> time. And I can see that this method is called:
>
> ~ 30.000 times in JFX 18
> ~ 45.000 times in JFX 19 (which is 3 iterations * 5000 rows more than 
> in JFX 18, as expected since we have an additional row whose css needs 
> to be applied)
> ~ 105.000 times in JFX 20
> ~ 105.000 times in JFX 21
>
>
> In looking at my changes for PR 757, I noticed that there are calls to 
> cell.updateTableColumn(tc) and cell.updateTableView(tv) that need not 
> be within the loop iterating over the rows. When putting these two 
> lines before the loop, I was able to measure:
>
> JFX 23-internal+0-2024-02-01-061822 average run time: 1119
>
> which would be roughly at the performance lebel of JFX 19 again.
>
> Interestingly, when profiling with this optimization I still get 105k 
> calls to transitionToState, but they seem to be much faster.

Is it still a CSS performance degradation then?  I mean, it's the same 
speed, and the number of calls should be irrelevant?

However, if you want to narrow it down further (perhaps there is more 
performance to be gained), you could run your tests against the early 
access builds.  You may be able to find a set of 10-20 isolated commits 
that could have led to the introduction of the extra 60.000 calls.  For 
example, check if the problem is present in 20-ea+1 up to 20-ea-19, 20, 
20.0.1 and 20.0.2 (early access versions available are 1, 2, 3, 4, 6, 7, 
9, 11, 19), see here: 
https://mvnrepository.com/artifact/org.openjfx/javafx-base

I checked the recent changes I did for CSS:

* CSS performance regression up to 10x 
(https://bugs.openjdk.org/browse/JDK-8322795)
* Quadratic layout time with nested nodes and pseudo-class in style 
sheet (https://bugs.openjdk.org/browse/JDK-8199216)
* Public API in javafx.css.Match should not return private API class 
PseudoClassState (https://bugs.openjdk.org/browse/JDK-8304959)
* Region#padding property rendering error 
(https://bugs.openjdk.org/browse/JDK-8245919)

However, none of those were present in JavaFX 20.

--John