The default bug view has changed. See this FAQ.

Add a layers transactions meter

RESOLVED FIXED in mozilla17

Status

()

Core
Graphics: Layers
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: cjones, Assigned: mattwoodrow)

Tracking

unspecified
mozilla17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

We've had some bugs where we were sending way too many layers transactions, but the throttling of composition was hiding those bugs.  Matt added a second rate counter for layers transactions, in addition to the compositor counter.  I'd like to land that.
Created attachment 648557 [details] [diff] [review]
Meter layers transactions in addition to composites
Attachment #648557 - Flags: review?(jones.chris.g)
Comment on attachment 648557 [details] [diff] [review]
Meter layers transactions in addition to composites

>diff --git a/gfx/layers/Layers.h b/gfx/layers/Layers.h

>+  virtual void NotifyContentUpdated() {}

More precisely: |NotifyShadowTreeTransaction()|.  I would prefer to
stick this method on ShadowLayerManager if it's not too inconvenient.

>diff --git a/gfx/layers/ipc/CompositorParent.cpp b/gfx/layers/ipc/CompositorParent.cpp

> void
>+CompositorParent::ContentUpdated()

Similarly, |NotifyShadowTreeTransaction()|.

>@@ -861,16 +868,17 @@ CompositorParent::ShadowLayersUpdated(Sh

>   ScheduleComposition();
>+  mLayerManager->NotifyContentUpdated();

Should this just call NotifyShadowTreeTransaction().

>diff --git a/gfx/layers/opengl/LayerManagerOGL.cpp b/gfx/layers/opengl/LayerManagerOGL.cpp

>+void
>+LayerManagerOGL::FPSState::ContentUpdated()

And here.

r=me with that.  Will file followup for ... interesting ... fps
calculation.
Attachment #648557 - Flags: review?(jones.chris.g) → review+
(Assignee)

Comment 3

5 years ago
Created attachment 649525 [details] [diff] [review]
Meter layers transactions in addition to composites

Fixed review comments, carrying forward r=cjones
Attachment #648557 - Attachment is obsolete: true
Attachment #649525 - Flags: review+
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9fb78d71f361
Assignee: nobody → matt.woodrow
Sorry, I had to back this out because it caused all native Android tests runs to crash/timeout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a525ac9349eb
I noticed that in

 void
+CompositorParent::NotifyShadowTreeTransaction()
+{
+  ShadowLayerManager *shadow = mLayerManager->AsShadowManager();
+  if (shadow) {
+    shadow->NotifyShadowTreeTransaction();
+  }
+  ScheduleComposition();
+}

we don't check if mLayerManager is null, which Composite() does.
https://tbpl.mozilla.org/?tree=Try&rev=32af6c0fdfc8
Created attachment 655388 [details] [diff] [review]
Rebased, with added null check
Andreas, if the try run in comment 7 comes back green while I'm in the air, this would be great to land before the work week to help with perf analysis.
Blocks: 780074
https://hg.mozilla.org/integration/mozilla-inbound/rev/dbc34849f109
https://hg.mozilla.org/mozilla-central/rev/dbc34849f109
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.