Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Enable leaf layer tracing in PaintContext#32383

Merged
fluttergithubbot merged 2 commits intoflutter:mainfrom
iskakaushik:leaf-layer-tracing
Apr 12, 2022
Merged

Enable leaf layer tracing in PaintContext#32383
fluttergithubbot merged 2 commits intoflutter:mainfrom
iskakaushik:leaf-layer-tracing

Conversation

@iskakaushik
Copy link
Contributor

@iskakaushik iskakaushik commented Apr 1, 2022

During performance debugging of a flutter application, it is useful to have additional data around how long subcomponents of the displayed frame contributed to the overall raster performance. Towards this goal, this change allows for a layer tree to request leaf-layer tracing. When this is enabled, the leaf layers are rendered to an off-screen texture and the texture is then stored in the LayerSnapshotStore.

In later changed, this stored snapshots will be transferred to the Flutter tool using a service protocol.

The snapshots for the display list layer are recorded in `LayerSnapshotStore`

This still needs to be wired via service protocol, but this patch
is independently valid.
@iskakaushik iskakaushik requested a review from dnfield April 1, 2022 15:03
@iskakaushik
Copy link
Contributor Author

cc: @gaaclarke

}

if (context.enable_leaf_layer_tracing) {
const auto canvas_size = context.leaf_nodes_canvas->getBaseLayerSize();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this be prohibitively expensive to do on a series of frames?

For example, what if instead of eagerly storing the rasterized leaf, we instead stored the leaf with enough information to rasterize it later.

Then, we could always record the time it takes to rasterize the whole layer tree. We could update the snapshot store with those times. Later, a user could request rasterizations of the top N frames with their broken down raster times (and the amount of time it actually took to rasterize the whole scene).

WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this approach would also let us keep DRM on when this is enabled, but maybe I'm misunderstanding.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently we expect enable_leaf_layer_tracing to be false, except when user types in j when the Flutter tool is attached. That way I'm not expecting to store more than 1 frames worth of data in LayerTree store ever. When we do make the change to print N frames worth of raster information, I think we should do something like storing the serialized display list somewhere on device and get the raster information out.


std::unique_ptr<FrameDamage> damage;
if (frame->framebuffer_info().supports_partial_repaint) {
// when leaf layer tracing is enabled we wish to repaing the whole frame for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo?

I also think this is probably going to make the performance metrics less helpful. For example, imagine you have a relatively expensive leaf (some kind of fancy drawing or SVG or something) that does not change from frame to frame. Enabling leaf layer tracing will make it artificially seem like that leaf layer is very expensive on every frame, when it's really only very expensive on the first one (maybe where you still have enough budget for it).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(please fix the typo before landing)

@gaaclarke
Copy link
Member

Can you expand the description please?

@iskakaushik
Copy link
Contributor Author

Can you expand the description please?

Can you expand the description please?

done!

Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

We should file some follow up bugs to improve this by extending it to worst-K frames, enabling or disabling DRM optionally

@iskakaushik iskakaushik added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Apr 11, 2022
@fluttergithubbot fluttergithubbot merged commit 9164243 into flutter:main Apr 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants