Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression in timeToFirstFrameRasterizedMicros on Mac_android_complex_layout__start_up #87085

Closed
zanderso opened this issue Jul 27, 2021 · 8 comments · Fixed by #87126
Closed
Assignees
Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) c: regression It was better in the past than it is now engine flutter/engine repository. See also e: labels. P0 Critical issues such as a build break or regression perf: speed Performance issues related to (mostly rendering) speed

Comments

@zanderso
Copy link
Member

From the mid-500s ms up to high 700s ms.

https://flutter-flutter-perf.skia.org/e/?begin=1624501862&end=1627353183&keys=Xc8b23a2e76f54575dead81d68735842f&requestType=0&xbaroffset=25119

Suspecting the engine roll here: #86825

Maybe flutter/engine#27629 ? /cc @dnfield

@zanderso zanderso added c: regression It was better in the past than it is now engine flutter/engine repository. See also e: labels. c: performance Relates to speed or footprint issues (see "perf:" labels) perf: speed Performance issues related to (mostly rendering) speed P2 labels Jul 27, 2021
@dnfield
Copy link
Contributor

dnfield commented Jul 27, 2021

I guess it's possible that creating the additional surface is causing this. I'll try to look.

@dnfield
Copy link
Contributor

dnfield commented Jul 27, 2021

I'm pretty sure it was the pbuffer commit.

I have an idea to fix forward that I'd like to test - if I can't prove it quickly I'll revert.

@dnfield dnfield self-assigned this Jul 27, 2021
@zanderso
Copy link
Member Author

@dnfield
Copy link
Contributor

dnfield commented Jul 27, 2021

So before the pbuffer patch, the ShaderWarmUp routine did not actually produce any shaders, because it ended up working with a raster based surface. Now, however, it does produce shaders.

@dnfield
Copy link
Contributor

dnfield commented Jul 27, 2021

Specifically, this line here:

final ui.Image image = await picture.toImage(size.width.ceil(), size.height.ceil());

@dnfield
Copy link
Contributor

dnfield commented Jul 27, 2021

I'm inclined to drop the default shader warmup routine from the framework - @zanderso @chinmaygarde WDYT?

Internal customers that are using this have their own shader warmup specializations. However, this never does anything on Android or iOS.

@dnfield
Copy link
Contributor

dnfield commented Jul 28, 2021

Both the memory and the time to first frame regressions improved after removing the default shader warm-up routine.

@github-actions
Copy link

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 -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 11, 2021
@flutter-triage-bot flutter-triage-bot bot added P0 Critical issues such as a build break or regression and removed P2 labels Jun 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) c: regression It was better in the past than it is now engine flutter/engine repository. See also e: labels. P0 Critical issues such as a build break or regression perf: speed Performance issues related to (mostly rendering) speed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants