779940 - Add a layers transactions meter

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

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.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Meter layers transactions in addition to composites

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

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.

Comment 3

[Matt Woodrow (:mattwoodrow)]

Attachment: Meter layers transactions in addition to composites

Fixed review comments, carrying forward r=cjones

Comment 4

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9fb78d71f361

Comment 5

[Matt Brubeck (:mbrubeck)]

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

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

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.

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

https://tbpl.mozilla.org/?tree=Try&rev=32af6c0fdfc8

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Rebased, with added null check

Comment 9

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

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.

Comment 10

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dbc34849f109

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/dbc34849f109