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

[Impeller] Investigate "DlCanvas implementation wrapping Aiks canvas" 90th percentile increase #132071

Closed
gaaclarke opened this issue Aug 7, 2023 · 8 comments
Labels
e: impeller Impeller rendering backend issues and features requests engine flutter/engine repository. See also e: labels. team-engine Owned by Engine team

Comments

@gaaclarke
Copy link
Member

flutter/engine#44248 seems to have improved the average rasterizer time for the backdrop filter benchmark, but potentially raised the floor on the gallery 90th percentile test (albeit the attribution is tenuous). Can we double check the implementation to make sure there isn't something we missed that might negatively effect the 90th percentile

backdrop filter average raster time benchmark drop

Screenshot 2023-08-07 at 10 39 39 AM

gallery 90th percentile increase

(notice that the floor never reaches 10ms around the time that PR landed)
Screenshot 2023-08-07 at 10 11 58 AM

cc @dnfield

@gaaclarke gaaclarke added the e: impeller Impeller rendering backend issues and features requests label Aug 7, 2023
@gaaclarke
Copy link
Member Author

I think it's easiest if @dnfield just gives it a look to see if there is some easy tweak we could make. He won't be available for a while though.

@jason-simmons
Copy link
Member

@gaaclarke
Copy link
Member Author

@zanderso @chinmaygarde what do you want to do about that PR? Looks like its benefits are complicated and I looked at the PR and it still had some unanswered comments on it. Dan isn't going to be available for a minute to investigate.

@zanderso
Copy link
Member

zanderso commented Aug 7, 2023

Let's revert. @dnfield can re-land next week after investigating the regressions noted here.

jason-simmons added a commit to jason-simmons/flutter_engine that referenced this issue Aug 7, 2023
auto-submit bot pushed a commit to flutter/engine that referenced this issue Aug 7, 2023
@danagbemava-nc danagbemava-nc added engine flutter/engine repository. See also e: labels. team-engine Owned by Engine team labels Aug 8, 2023
@zanderso
Copy link
Member

zanderso commented Aug 8, 2023

The improvement in the drawPoints benchmark is so large as to be suspicious. We should verify that it was still rendering something with the change.

@chinmaygarde
Copy link
Member

The revert has landed and I have filed an issue tracking re-landing with references #132416. Closing this.

@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 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
e: impeller Impeller rendering backend issues and features requests engine flutter/engine repository. See also e: labels. team-engine Owned by Engine team
Projects
No open projects
Archived in project
Development

No branches or pull requests

6 participants