Closed Bug 1017353 Opened 6 years ago Closed 6 years ago

Layer transaction containing 120 layers take a 30-40ms processing transaction update, updating layer regions

Categories

(Core :: Graphics: Layers, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32
Tracking Status
b2g-v2.0 --- fixed

People

(Reporter: BenWa, Unassigned)

References

Details

Stack:
mozilla::layers::LayerPropertiesBase::ComputeChange(void (*)(mozilla::layers::ContainerLayer*, nsIntRegion const&), bool&)
mozilla::layers::ContainerLayerProperties::ComputeChangeInternal(void (*)(mozilla::layers::ContainerLayer*, nsIntRegion const&), bool&)
mozilla::layers::LayerPropertiesBase::ComputeChange(void (*)(mozilla::layers::ContainerLayer*, nsIntRegion const&), bool&)
mozilla::layers::ContainerLayerProperties::ComputeChangeInternal(void (*)(mozilla::layers::ContainerLayer*, nsIntRegion const&), bool&)
mozilla::layers::LayerPropertiesBase::ComputeChange(void (*)(mozilla::layers::ContainerLayer*, nsIntRegion const&), bool&)
mozilla::layers::LayerPropertiesBase::ComputeDifferences(mozilla::layers::Layer*, void (*)(mozilla::layers::ContainerLayer*, nsIntRegion const&), bool*)
mozilla::layers::LayerManagerComposite::EndTransaction(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags)
mozilla::layers::LayerTransactionParent::RecvUpdate(nsTArray<mozilla::layers::Edit> const&, mozilla::layers::TargetConfig const&, bool const&, bool const&, unsigned int const&, nsTArray<mozilla::layers::EditReply>*)
LayerTransactionParent::RecvUpdate
mozilla::layers::LayerTransactionParent::RecvUpdateNoSwap(nsTArray<mozilla::layers::Edit> const&, mozilla::layers::TargetConfig const&, bool const&, bool const&, unsigned int const&)
mozilla::layers::PLayerTransactionParent::OnMessageReceived(IPC::Message const&)
IPDL::PLayerTransaction::RecvUpdateNoSwap
mozilla::layers::PCompositorParent::OnMessageReceived(IPC::Message const&)
mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&)
mozilla::ipc::MessageChannel::OnMaybeDequeueOne()
DispatchToMethod<WebCore::ReverbConvolver, void (WebCore::ReverbConvolver::*)()>
mozilla::ipc::MessageChannel::RefCountedTask::Run()
MessageLoop::RunTask(Task*)
MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&)
MessageLoop::DoWork()
base::MessagePumpDefault::Run(base::MessagePump::Delegate*)
MessageLoop::RunInternal()
MessageLoop::RunHandler()
base::Thread::ThreadMain()
(root)

Profile:
http://people.mozilla.org/~bgirard/cleopatra/#report=221a61f50bb0ffaa1c107e1a1c5b5f2046eb80bb

(Ignore the layers.dump that's enabled, focus on LayerManagerComposite::EndTransaction.
I thought we only needed the ComputeDifferences in order to know which areas the BasicCompositor needs to recomposite. Can we skip this for other compositor types?
Sounds like a job for SimplifyOutward-man!
This was introduced by bug 882447.
Depends on: 882447
And this was turned on for OGL in bug 965102.
Depends on: 965102
(In reply to Markus Stange [:mstange] from comment #1)
> I thought we only needed the ComputeDifferences in order to know which areas
> the BasicCompositor needs to recomposite. Can we skip this for other
> compositor types?

B2G, when using the hardware composer wants to know if the geometry of the layer tree has changed in order to make some optimizations. Maybe running the full ComputeDifferences is too heavyweight for this. We also can disable it for systems where we don't have HWC.
Jeff and I were just discussing if we should disable it elsewhere. Its a lot of extra work to do it but the place where we can't afford to do it is on B2G. We should instead focusing on making it cheaper.
Depends on: 1018416
Benwa, I am just curious, which use case has 120 layers, is it (Container + Leaf layers) = 120 OR number of leaf layers = 120 ? Please let me know.

Also, we anyway cannot use HWC if the number of leaf layers is too high due to limitation on number of H/W resources. So we can add a check on number of children (on Root), before calling ComputeDifferences() in  LayerManagerComposite::EndTransaction(). This will make sure that we do not call ComputeDifferences() when # of layers is too high.
Flags: needinfo?(bgirard)
(In reply to Sushil from comment #7)
> Benwa, I am just curious, which use case has 120 layers, is it (Container +
> Leaf layers) = 120 OR number of leaf layers = 120 ? Please let me know.

Bug 1015984  has a testcase. 120 animated elements isn't that much. We should be able to handle it gracefully.

> 
> Also, we anyway cannot use HWC if the number of leaf layers is too high due
> to limitation on number of H/W resources. So we can add a check on number of
> children (on Root), before calling ComputeDifferences() in 
> LayerManagerComposite::EndTransaction(). This will make sure that we do not
> call ComputeDifferences() when # of layers is too high.

The real problem is that code is just too slow.
Flags: needinfo?(bgirard)
I did some profiling and this problem is now fixed.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
OK, I get it, this one, not bug 1015984.  Never mind :)
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.