-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Add profiling counts to pipeline uses #179374
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
Add profiling counts to pipeline uses #179374
Conversation
3c82621 to
73db6a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This PR adds profiling for pipeline variant usage in Impeller. It introduces a mechanism to track pipeline creation and usage, and exposes this data through a new service protocol extension. The changes span across the rendering backends (Metal, GLES, Vulkan) to log pipeline usage, and modify the shell to handle the new service protocol request.
My review focuses on correctness, thread safety, and code improvements. I've found a critical data race in PipelineLibrary due to unsynchronized access to the usage counts map from multiple threads. I've also identified a typo in the new service protocol extension name, and suggested some code simplifications and optimizations. Please see the detailed comments.
chinmaygarde
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the late review! Some minor suggestions.
|
autosubmit label was removed for flutter/flutter/179374, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
This PR adds functionality that logs uses of pipeline variants during render passes across all rendering backends for impeller. It also adds a method to the engine service protocol that emits these usage statistics. New functionality is unit tested.
Fixes #176660.