Repository navigation
2x regression in total_ui_gc_time for backdrop_filter_perf_ios__timeline_summary #159337
Description
Apparent introduction: #159072
{{ message }}
2x regression in
Activity
gaaclarke commentedon Nov 22, 2024
@justinmc can you take a look at this regression please? It seems something in that PR has increased the garbage are generating. Maybe there is some way we can tweak it to make less garbage?
matanlurey commentedon Nov 22, 2024
I took a quick look before a meeting - there is a significant amount of
Sets being allocated and filled perbuild()now that were not before - if I had to guess it is related to that.goderbauer commentedon Nov 26, 2024
@bleroux Maybe we should revert that change?
bleroux commentedon Nov 27, 2024
@goderbauer Thanks for pinging me.
I filed #159546 which optimizes the Set computations added in #159072.
Nonetheless, it is not clear to me why this change would impact this particular benchmark because there are no InkWell involved in this benchmark.
The same graph for Android was not impacted, see https://flutter-flutter-perf.skia.org/e/?begin=1732616899&end=1732703299&queries=sub_result%3Dtotal_ui_gc_time%26test%3Dbackdrop_filter_perf__timeline_summary.
Could it be possible that a change merged some hours before my PR leads to this regression?
I ask this because this benchmark uses
Image.blurintensively and the following engine roll landed some hours before my PR and it contains a blur related change.To see if this theory is valid, we should probably revert #159072 (and before revert #159203 which depends on it).
Doing so we will see, if the perf regression is gone or not.
Let me know it it makes sense. If it does, thanks to initiate the revert process by starting with #159203 and then #159072.
gaaclarke commentedon Nov 27, 2024
I don't think so, but I think you are right to be skeptical that your PR caused this change. I saw similar regressions as a result of #159228. That PR made the framerate faster on iOS, so more garbage was being generated per time. The weird thing is that that PR landed a day after yours did.
Revert "Fix Date picker overlay colors aren't applied on selected sta… (
Revert "Fix InkWell overlayColor resolution ignores selected state (#…
zanderso commentedon Dec 2, 2024
It looks like the flagged change was reverted, so I'll go ahead and close this. Feel free to re-open if I'm mistaken.
bleroux commentedon Dec 3, 2024
@gaaclarke I reverted my PR several days ago (last Tuesday). Due to thanksgiving there were few commits until Monday. Now we have some data and the regression seems to be still there (my PR was flagged with a total_gc_gpu_time of 1.5, but since my revert there is one occurrence at 2.03, one at 1.66 and several significantly above the previous average.
I don't know the policy about tackling those perf regressions, especially if the issue should be reopened.
gaaclarke commentedon Dec 3, 2024
Okay, thanks for doing that. I would go ahead and reland your PR and not worry about it. I'm not sure what is going on. Usually we try to catch stuff as it comes it. It becomes difficult to manage otherwise. This time we got weird signals though. FWIW GC time is kind of a secondary metric; like I mentioned, faster frame times can lead to its increase, which is actually a good thing. We'll keep an eye on it.
Reland Fix InkWell overlayColor resolution ignores selected state (#1…
github-actions commentedon Dec 17, 2024
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of
flutter doctor -vand a minimal reproduction of the issue.