If you look closely, I removed:
tableSkin.getChildren().remove(cell);
So the cell was previously removed, but it was never added to the table skin in the first place.
The cell is added to the row instead.
Thats why the line below does remove the table row instead:
tableSkin.getChildren().remove(tableRow);
So your code is faster because it will remove the table row in the first loop iteration and never add it again, so the call to applyCss() is faster (basically a noop as the row is not inside the scenegraph).
-- Marius
Gesendet: Freitag, 02. Februar 2024 um 10:49 Uhr
Von: "Robert Lichtenberger" <r.lichtenber...@gmail.com>
An: openjfx-dev@openjdk.org
Betreff: Re: CSS Performance regression inTableColumnHeader.resizeColumnToFitContent
Von: "Robert Lichtenberger" <r.lichtenber...@gmail.com>
An: openjfx-dev@openjdk.org
Betreff: Re: CSS Performance regression inTableColumnHeader.resizeColumnToFitContent
Am 02.02.24 um 07:10 schrieb Robert Lichtenberger:
>
>>
>> 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
>
> Good idea, I will try that.
Okay, using prebuilt ea-Versions I was able to narrow the problem down:
JFX 19.0.2+1 average run time: 1113
JFX 20-ea+1 average run time: 1110
JFX 20-ea+7 average run time: 1211
JFX 20-ea+9 average run time: 1115
JFX 20-ea+11 average run time: 1646
JFX 20.0.2+3 average run time: 1656
=> Something has happened between ea+9 and ea+11.
I've then let the benchmark run against self-built sdks that are based
on the git tags.
git checkout tags/20+9: JFX 20-internal+0-2024-02-02-095926 average run
time: 1098
git checkout tags/20+11: JFX 20-internal+0-2024-02-02-095926 average run
time: 1576
So this confirms the data of the prebuilt sdks.
There's also a 20+10 tag, which is still fast: JFX
20-internal+0-2024-02-02-095926 average run time: 1078
So now I've looked at the commits betweeen 20+10 and 20+11.
git checkout c2af0c31b70df238769bd0eb1b7fd04eb7241446: JFX
20-internal+0-2024-02-02-095926 average run time: 1093
git checkout bb98d886b01d5d1c6117303f40d43eab9f7ac504: JFX
20-internal+0-2024-02-02-095926 average run time: 1072
=> So it wasn't the "8295806: TableViewSkin: memory leak when changing
skin" commit
git checkout 6abbe0803456ad648117b8e72deeeeced7cb5231: JFX
20-internal+0-2024-02-02-095926 average run time: 1086
git checkout c900a00c7527f290e8047792fef4b45002930892: JFX
20-internal+0-2024-02-02-095926 average run time: 1608
=> So the "culprit" seems to be the "8289357: (Tree)TableView is null in
(Tree)TableRowSkin during autosize" commit by Marius Hanl.
Looking into the commit, I see that the
"tableSkin.getChildren().remove(tableRow);" hast been moved out of the
for-loop.
I've put it back into the loop, which should make things slower, but:
JFX 20-internal+0-2024-02-02-095926 average run time: 161
Looks like things are _ten times_ faster this way.
Tests in javafx.controls still pass.
Sounds almost too good to be true, so I will investigate a bit further...
Robert
