1441308 - (document-splitting) WebRender: Document splitting

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement, P3)

Description

[Jeff Muizelaar [:jrmuizel]]

We have multiple proposals for this:

A:
 - Split the Gecko display list into documents when remote iframe's are encountered.
 - This will require some trickier code to do.

B:
 - Only remote iframe can be new documents
 - This means the status bar must be moved into a new remote iframe.
 - This means a DOM document can't have content above and below a WebRender Document.

We don't yet know which is best.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

Some more spit balling:

C: Sort of an iteration on B. Add a some kind of new frame type (WRDoc) that corresponds to a webrender document. During paint we build separate wr display lists under each WRDoc frame. We also gather information about the position and order of the WRDoc frames and communicate that to WebRender so that the documents can go in the right locations.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Also something to keep in mind is that with the fission project the number of remote iframes is going to go up dramatically, so if we have one webrender document per remote iframe then we'll end up with a lot of webrender documents. That might be desirable, or it might not be.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

I talked option C over with mstange and he thought it would be reasonable. In the tagged frame we would add a new wrapping display item that would note the document. As we're building the WebRender display list we would just keep track of our current document as we traverse and switch it as appropriate.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

I've pointed dthayer at this - let's see where it goes!

Comment 5

[Alex Thayer [:alexical] (she/her)]

Summing up a conversation with Markus and Jeff, everything above the content-deck is going to get its own document, and everything inside content-deck and below will get the same document. I.e., the status bar / side bar / devtools / etc. will all share a document with the content. At present we'll just render both documents into the default framebuffer with different offsets.

Also, "document" is kind of confusing since it's an overloaded word. We're thinking of going with "stage" instead?

Comment 6

[Bobby Holley (:bholley)]

(In reply to Doug Thayer [:dthayer] from comment #5)
> Summing up a conversation with Markus and Jeff, everything above the
> content-deck is going to get its own document, and everything inside
> content-deck and below will get the same document. I.e., the status bar /
> side bar / devtools / etc. will all share a document with the content. At
> present we'll just render both documents into the default framebuffer with
> different offsets.
> 
> Also, "document" is kind of confusing since it's an overloaded word. We're
> thinking of going with "stage" instead?

"stage" feels kinda vague too. What about something like RenderRoot?

Comment 7

[Jeff Muizelaar [:jrmuizel]]

Here's the current try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4d4c82731b3f14784c085a992a1e515c0c265183

Comment 8

[Alex Thayer [:alexical] (she/her)]

The bulk of the work can be found here: https://hg.mozilla.org/try/rev/02c1618e5bed

To simplify the initial implementation, I hard-coded it to two documents - the chrome document, for everything in the chrome area above the content, and the content document, for everything underneath. The content document includes the content but also the devtools, the sidebar if it exists, and anything else that occupies that space. To delineate this I added a "webrendercontent" attribute which gets added to the "content-deck" here[1]. For the most part the document that we're intending to use for a particular thing is just described by a boolean like "aIsForContent"[2]. In the future, we can expand this to support N documents rather than just 2, but it was simplest to first implement it this way and expand if necessary.

The overall strategy was to start building the chrome WR display list and, if we encounter a special display item (nsDisplayRenderRoot), we switch to building the content display list. This is implemented by housing a pointer to the content DisplayListBuilder inside the chrome DisplayListBuilder, and switching them out inside nsDisplayRenderRoot's CreateWebRenderCommands implementation[3]. I also split the IPCResourceUpdateQueue and StackingContextHelper in a similar way for similar reasons. Additionally, from the content process we only build for the content document.

We send these split display lists off to the WebRenderBridgeParent through duplicated parameters - one set of parameters for the chrome document, and one for the content document. We could try to implement this by adding a boolean and sending them as separate messages, but there seem to be ordering problems with doing so and there's bookkeeping that the WebRenderBridgeParent does which becomes complicated when it's getting, say, two RecvSetDisplayList calls per logical update.

Unfortunately not all updates are done from the scope of CreateWebRenderCommands, so in some cases additional state needs to be stored on objects so they can appropriately add their updates to either the chrome document or the content document's command buffer[4].

Additionally, we want to avoid as much unnecessary work as possible, so we check HasAnyStateBits(NS_FRAME_NEEDS_PAINT | NS_FRAME_DESCENDANT_NEEDS_PAINT) when creating the nsDisplayRenderRoot display item, and we avoid sending the display list accordingly. Unfortunately, in the linked patch I'm still building the display lists, and only avoiding sending them. This is because we create the APZ WebRenderScrollData in the course of creating the WR display lists, and the APZUpdater clobbers the previous WebRenderScrollData in the process of updating, and I haven't been able to split it out by document yet, so if the content document doesn't build this and send it up then we run into trouble.

Lastly, I am using WebRenderBridgeParent's mPipelineId right now as the root pipeline id for both the chrome and the content documents. This is because this ID is derived from CompositorSession::mRootLayerTreeId, which holds meaning outside of WebRender. It's mostly fine to have the same mPipelineId for both documents, as WR doesn't impose any requirement that they be unique, except for the fact that WR epochs are tracked in a hash map keyed on pipeline ID, and so the content and chrome documents can clobber each other's epochs. I'm investigating solutions to this right now, but suggestions / questions are welcome.

[1] https://hg.mozilla.org/try/rev/02c1618e5bed#l1.13
[2] https://hg.mozilla.org/try/rev/02c1618e5bed#l9.14
[3] https://hg.mozilla.org/try/rev/02c1618e5bed#l52.125
[4] https://hg.mozilla.org/try/rev/02c1618e5bed#l9.17, https://hg.mozilla.org/try/rev/02c1618e5bed#l27.13, etc.

Comment 9

[Alex Thayer [:alexical] (she/her)]

Sotaro, since you work in some of this space, would you mind reading over the summary and note if anything sounds like it's going in the wrong direction? The overall goal is just to be able to split out the work we do for the chrome area from the content area, so that we don't have to build or send display lists for things in the content area when only the chrome area changes and vice versa. Additionally, as a follow-up we would like to be able to draw these into separate framebuffers and composite them with the OS compositor.

Please let me know if you have any questions!

Comment 10

[Sotaro Ikeda [:sotaro]]

Thanks for the detailed summary! I am looking into https://hg.mozilla.org/try/rev/02c1618e5bed.

Comment 11

[Alex Thayer [:alexical] (she/her)]

Just chatted with Matt Woodrow about the HasAnyStateBits(NS_FRAME_NEEDS_PAINT | NS_FRAME_DESCENDANT_NEEDS_PAINT) check to determine if we can skip building the content area WR display list. Unfortunately this doesn't cover what we need and the only thing that does is behind retained-dl, which is disabled in the chrome right now.

So my current plan is to update this to use the retained-dl information, and gate doc splitting behind retained-dl. Then I can try to prove out the perf gains of doc splitting to justify finalizing chrome retained-dl. Unfortunately I suspect really proving out the perf gains of doc splitting will require painting the documents to separate framebuffers and compositing them with the OS compositor.

Comment 12

[Sotaro Ikeda [:sotaro]]

I built https://hg.mozilla.org/try/rev/02c1618e5bed and it worked well except the following build failure. It is great!
  https://hg.mozilla.org/try/rev/02c1618e5bed#l50.36

We need to keep current WebRenderLayerManager for layout(Refresh Driver, etc), but needs to support multiple documents.Then current approach looks reasonable. But the change is very big than I expected. We need to make the change as cleaner and simpler as possible.

Some comments.

-[1] I do not know well about StackingContextHelper. You might want to ask :kats about it.

-[2] There are a lot of bool IsForContentRect flag. It might be better to use "Document type enum" instead of bool.
     It make an intent of the code clearer.

-[3] The IsForContentRect flag is used for ImageKeys handling like SharedSurfacesChild, but it seems not necessary.
   WebRender resources does not belong to WebRender document. The Update reource belongs to enum ApiMsg.
   https://dxr.mozilla.org/mozilla-central/source/gfx/webrender_api/src/api.rs#672

-[4] Some WebRenderBridgeParent functions like RecvUpdateResources() uses aIsForContentRect flag to choose api.
  like [3], WebRender resources does not belong to WebRender document. Any wr api should be OK.

-[5] Some function calls in WebRenderBridgeParent looks redundant.
  For example, WebRenderBridgeParent::FlushSceneBuilds() calls FlushSceneBuilder() twice like the following.
  The function triggers task that does not belong to document. Then we do not need to call twice.

 >  mApi->FlushSceneBuilder();
 >  mContentRectApi->FlushSceneBuilder();

-[6] aSkipFrameNotification of ContentFrame in WebRenderBridgeParent::MaybeGenerateFrame() seems not work well with current WebRender.
  The MaybeGenerateFrame() does not know in advance if new_frame_ready() will request rendering.
  Current WebRender might skip frame rendering since Bug 1461239 fix.

Comment 14

[Alex Thayer [:alexical] (she/her)]

Still working on cleaning this up, but I did have some responses to a few of these.

(In reply to Sotaro Ikeda [:sotaro] from comment #12)
> -[1] I do not know well about StackingContextHelper. You might want to ask
> :kats about it.
> 
> -[2] There are a lot of bool IsForContentRect flag. It might be better to
> use "Document type enum" instead of bool.
>      It make an intent of the code clearer.
> 
> -[3] The IsForContentRect flag is used for ImageKeys handling like
> SharedSurfacesChild, but it seems not necessary.
>    WebRender resources does not belong to WebRender document. The Update
> reource belongs to enum ApiMsg.

The resources do not belong to WebRender documents, but because we want to be able to generate frames for chrome and content documents independently, we still need this. Right now we use whether there were any resource updated to determine if we want to generate a frame or not, and in these cases we would have to generate both chrome and content frames if we didn't differentiate ownership between resources.

> https://dxr.mozilla.org/mozilla-central/source/gfx/webrender_api/src/api.
> rs#672
> 
> -[4] Some WebRenderBridgeParent functions like RecvUpdateResources() uses
> aIsForContentRect flag to choose api.
>   like [3], WebRender resources does not belong to WebRender document. Any
> wr api should be OK.

See above.

> 
> -[5] Some function calls in WebRenderBridgeParent looks redundant.
>   For example, WebRenderBridgeParent::FlushSceneBuilds() calls
> FlushSceneBuilder() twice like the following.
>   The function triggers task that does not belong to document. Then we do
> not need to call twice.

Yeah I think you're right. I'm still going through all of this and cleaning up. Many things were somewhat coarsely duplicated.

> 
>  >  mApi->FlushSceneBuilder();
>  >  mContentRectApi->FlushSceneBuilder();
> 
> -[6] aSkipFrameNotification of ContentFrame in
> WebRenderBridgeParent::MaybeGenerateFrame() seems not work well with current
> WebRender.
>   The MaybeGenerateFrame() does not know in advance if new_frame_ready()
> will request rendering.
>   Current WebRender might skip frame rendering since Bug 1461239 fix.

Good call! I'll work on a more robust solution.

Comment 15

[Alex Thayer [:alexical] (she/her)]

Jeff, given the dependency mentioned in comment 11. My current plan is to get this reviewed and landed behind a pref before I go on paternity leave (for another 4 weeks - beginning after mozlando). After I come back, or if anyone else wants to take it over, I can look into exploring the possibilities of integrating this more deeply with OS compositors to prove out the performance wins, along with helping to hammer out any of the remaining kinks with getting retailed-dl on in the chrome.

Thoughts?

Comment 16

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Add DocumentId to FrameId r=bholley

In order for multiple documents to share caches that use
FrameIds as a clock, FrameIds need to be document-aware,
otherwise documents which are not updating can have their
cached resources expired out from underneath them.

Hash table lookups were the simplest way I could drum up
to accomplish this, but I'm certainly open to suggestions.
I couldn't profile any difference in the single-document
case, so I didn't put much more into it.

Comment 17

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Add document cleanup methods to caches r=bholley

Fairly straightforward. Now that caches are document-aware,
we need to clean up document-specific bits when documents
are removed. We might want to avoid this when we're removing
documents as part of removing a whole WebRenderAPI.

Depends on D12840

Comment 18

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Move end_frame calls out a step r=bholley

Since we're now clearing the interal frame_id on caches at
the end of frames, we need to push the end_frame back a bit
in order for all uses of frame_id to still be valid.

Depends on D12841

Comment 19

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Only update epochs forward r=kats

With multiple documents sharing the same root pipeline ID
and updating independently, epochs can get out of sync.
In the cases where we use these epochs though we seem to
be able to just check the latest epoch, since we just want
to know if something that we've requested has been completed.

This does carry an assumption with it that anything using
documents will update epochs using a global counter, rather
than a per-document counter, which we may want to make
explicit, but I'm not certain of the best way to do that.

Depends on D12842

Comment 20

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Add the render root splitting pref r?sotaro

Depends on D12843

Comment 21

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Render Root Splitting WebRenderBridgeParent changes r=sotaro

The changes are a bit coarse, but it is difficult to split them
out logically. This covers the parent and ipdl changes for splitting
a window into two documents, one for the chrome area and one
for the content area.

Depends on D12844

Comment 22

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Render Root splitting child-side changes r=sotaro

This is the core of the changes on the child side of things. It
handles looking for the webrendercontent attribute and splitting
off a nsDisplayRenderRoot if necessary, which will split off a
separate WR display list.

Depends on D12845

Comment 23

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Stacking context handling r=kats

I wasn't altogether sure around how far this is necessary. I
know we need a stacking context and trial and error suggested
that I needed to create it at the top level and pass it all
the way down, but I'm not certain to what extent we need to
be duplicating the tree all the way down. Feedback is welcome.

Depends on D12846

Comment 24

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - AsyncImagePipelineManager changes r=sotaro

These changes largely revolve around ensuring that we know which
document a pipeline tracked by the AsyncImagePipelineManager
belongs to, so we can know what WR document needs to be updated.

Depends on D12847

Comment 25

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Ensure frame generation is scoped to relevant document r=sotaro

This ensures that frame generation from the scene builder is
sent to the appropriate document.

Depends on D12848

Comment 26

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Render Root splitting APZ changes r=kats

A few of these changes are self-explanatory, but the big
one that still has a TODO on it is what to do with the
aScrollUpdates parameter that normally gets passed through
RecvSetDisplayList. The problem is that we overwrite the
WebRenderScrollData for both the content and the chrome
documents when the root pipeline is updated, even though
the content could have been left out of that update since
it was not changed.

My idea is to merge the old content document parts of the
WebRenderScrollData into the new one if we know that we
skipped updating content, but I wanted to get feedback
on this approach before diving too deep.

Depends on D12849

Comment 27

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Allow for generating frames for multiple documents r=sotaro

This allows us to generate frames for multiple documents, but
only actually render once.

Depends on D12850

Comment 28

[Alex Thayer [:alexical] (she/her)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7193bd8a426c5acf15f340dcb273f9977167467

Comment 29

[Alex Thayer [:alexical] (she/her)]

Just to confirm, the problem listed in bug 1506435 is resolved by these patches with layout.display-list.retain.chrome on. Pages that jank the chrome by being complex no longer jank the chrome with these patches (yay!). Except, they still jank the tab bar. This is due to a problem on the retained-dl side that I believe is unnecessary and thus fixable.

Comment 30

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Make WR caches document-aware r?bholley

This change makes the various WR caches segment their cached data by
document, so that documents' data are not evicted out from underneath them.

Comment 31

[Alex Thayer [:alexical] (she/her)]

I'm not certain what to do with this regarding landing it. I will be on paternity leave after the all hands in Orlando, but my goal was to simply land it preffed off right at the beginning of my leave to avoid any severe merge conflicts, and to be available to aid in rolling it back should the need arise.

An alternative to this is that someone else takes it over. If anyone else is willing, I would be happy to offload any knowledge I have of the subject.

There seem to be three remaining issues with the patch. All of these only arise when the change is preffed ON, I can't observe any issues with doc splitting preffed off.

1. On Linux (can't observe on Windows or OSX), if I add ~10 new tab pages, some of the items that should be on the page disappear, beginning with pieces of the search input, then as I add more new tab pages the top site icons start disappearing. I've tried messing with cache eviction to simply never remove any cache items, and that doesn't seem to affect it. Still diagnosing.

2. Also only on Linux, when scrolling down a page, the scrolling occasionally jerks back and flashes content that should be scrolled off page. Previously I encountered similar issues that were due to epochs clashing due to the root pipeline ID being shared across the two documents. There might be remaining issues with epochs, or it could be something else. Any insights would be appreciated.

3. There's a TODO waiting on feedback from kats in the APZUpdater, regarding what to do with the mScrollData tree when we update the chrome area without updating the content area document, since the two share the tree of WebRenderScrollData. I think I have a reasonable plan for this but I'm not 100% certain.

In any case it's only worth preffing this on if we can also turn on layout.display-list.retain.chrome. So my hope was to land it preffed off and then assist in any of the remaining work to get chrome retained display lists into a good state. It already demonstrates big wins in the responsiveness of the chrome area on complex pages (see bug 1506435), and it could have additional wins if we move documents into separate framebuffers that we pass to the OS compositor.

Comment 32

[Kartikaya Gupta (email:[e-mail])]

Sorry for the delay reviewing, it's a big change and I need to carve out some time to page it all in. I'll try to get it done before Orlando so that if there are any major issues we can discuss during the week, or if not you can land the changes and we can monitor for fallout.

Comment 33

[Alex Thayer [:alexical] (she/her)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #32)
> Sorry for the delay reviewing, it's a big change and I need to carve out
> some time to page it all in. I'll try to get it done before Orlando so that
> if there are any major issues we can discuss during the week, or if not you
> can land the changes and we can monitor for fallout.

No worries! Thanks for the update :)

Also, jrmuizel requested one big patch to see all the changes - if anyone else needs that, you can see it here:
https://hg.mozilla.org/try/rev/7dd0d6a0ed41604b607865fbd5be15d2d89e8697

Comment 34

[Jeff Muizelaar [:jrmuizel]]

--- a/gfx/layers/wr/WebRenderLayerManager.h
+++ b/gfx/layers/wr/WebRenderLayerManager.h
@@ -128,20 +128,21 @@ public:
 
   wr::IpcResourceUpdateQueue& AsyncResourceUpdates();
   void FlushAsyncResourceUpdates();
 
   void RegisterAsyncAnimation(const wr::ImageKey& aKey, SharedSurfacesAnimation* aAnimation);
   void DeregisterAsyncAnimation(const wr::ImageKey& aKey);
   void ClearAsyncAnimations();
@@ -186,17 +187,19 @@ private:
    * Take a snapshot of the parent context, and copy
    * it into mTarget.
    */
   void MakeSnapshotIfRequired(LayoutDeviceIntSize aSize);
 
 private:
   nsIWidget* MOZ_NON_OWNING_REF mWidget;
   nsTArray<wr::ImageKey> mImageKeysToDelete;
+  nsTArray<wr::ImageKey> mContentRectImageKeysToDelete;
   nsTArray<wr::BlobImageKey> mBlobImageKeysToDelete;
+  nsTArray<wr::BlobImageKey> mContentRectBlobImageKeysToDelete;
 
   // Set of compositor animation ids for which there are active animations (as
   // of the last transaction) on the compositor side.
   std::unordered_set<uint64_t> mActiveCompositorAnimationIds;
   // Compositor animation ids for animations that are done now and that we want
   // the compositor to discard information for.
   nsTArray<uint64_t> mDiscardedCompositorAnimationsIds;
 
@@ -230,16 +233,17 @@ private:
   // See equivalent field in ClientLayerManager
   APZTestData mApzTestData;
 
   TimeStamp mTransactionStart;
   nsCString mURL;
   WebRenderCommandBuilder mWebRenderCommandBuilder;
 
   size_t mLastDisplayListSize;
+  size_t mLastContentDisplayListSize;
 
   Maybe<wr::IpcResourceUpdateQueue> mAsyncResourceUpdates;
   std::unordered_map<uint64_t, RefPtr<SharedSurfacesAnimation>> mAsyncAnimations;
 };

I think it makes sense to group the per document state and have it owned by the layer manager. This will let other parts of the pipeline refer to it directly instead of the pair of (LayerManager, RenderRoot). For example WebRenderUserData's could be changed to just point to the DocumentState instead of the LayerManager.

Comment 35

[Alex Thayer [:alexical] (she/her)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #34)
> I think it makes sense to group the per document state and have it owned by
> the layer manager. This will let other parts of the pipeline refer to it
> directly instead of the pair of (LayerManager, RenderRoot). For example
> WebRenderUserData's could be changed to just point to the DocumentState
> instead of the LayerManager.

Quick update on this. To avoid just replacing the (LayerManager, RenderRoot) pair with a (LayerManager, DocumentState) pair, DocumentState has to either take over more of LayerManager's managerie of responsibilities, or proxy to it for those responsibilities. I've taken the route of having it take over those responsibilities rather than proxy them for right now, but it's unclear to me how far the infection will spread, and whether we'll just end up replacing most of the references to WebRenderLayerManager with references to DocumentState. That might not be so bad, but it's a lot of function signatures to change.

Comment 36

[Jeff Muizelaar [:jrmuizel]]

(In reply to Doug Thayer [:dthayer] from comment #35)
> 
> Quick update on this. To avoid just replacing the (LayerManager, RenderRoot)
> pair with a (LayerManager, DocumentState) pair, DocumentState has to either
> take over more of LayerManager's managerie of responsibilities, or proxy to
> it for those responsibilities. I've taken the route of having it take over
> those responsibilities rather than proxy them for right now, but it's
> unclear to me how far the infection will spread, and whether we'll just end
> up replacing most of the references to WebRenderLayerManager with references
> to DocumentState. That might not be so bad, but it's a lot of function
> signatures to change.

FWIW, I imagined it taking over those responsibilities. I hope it works out :)

Comment 37

[Kartikaya Gupta (email:[e-mail])]

Ok, so I looked over the patchset to get an idea of how things work generally. I think in order to make this work well with APZ we will probably want to treat render roots similarly to how we treat layer ids. e.g. instead of just using the LayersId as the key at [1] and [2], we should use the (LayersId, RenderRoot) tuple as the key.

Basically, the problem as I see it is when the content process sends display lists for both render roots in a transaction. These are assigned the same epoch on the parent side, but are processed in separate documents, and therefore the scene builds in WR can happen in either order. So, we might get the post-scene-build notification to APZUpdater with the next epoch after the chrome display list is processed, but at that point the contentrect display list has not yet been processed. If we allow APZUpdater to go ahead and process all the messages for that LayersId, that will result in the APZ state getting updated to something that is out of sync with what WR has internally (because it will have processed the DL from one render root but not the other). So subsequent hit tests could fail because WR will be doing hit-tests against the old content scene and APZ has its internal state reflecting the new content scene.

This is basically the same problem we had to solve for display lists from different layers IDs, and I think it would be relatively straightforward to extend that solution to cover separate render roots as well. The hardest part will be making sure that the WebRenderScrollData items for the contentrect portion can be attached properly, but I think it's doable. The main problem here is the chain of FrameMetrics that needs to be maintained, which is actually very similar in nature to the chain of StackingContexts, and can probably be dealt with in a similar way.

With respect to the stacking context helper stuff, the general idea you have makes sense, I think. I think your current implementation will also generate spurious stacking contexts in the contentrect display list, because it will do so for stacking contexts that are not on the path to the content render root, but that should be relatively easy to fix - we can prevent the sub-context-helper from actually pushing anything to the builder until we hit the render root, at which point it can flush all the sub-context-helpers up the chain (would need to add parent links, but that's not hard). This modification would also allow us to squash together the stacking contexts if possible.

Overall I think much of the "visible" complexity (in terms of reading the code) in the patch actually comes from hard-coding the two render roots in a lot of places. If we made it more generic to take an arbitrary number of render roots I suspect it might actually make the code easier to read and follow. But at least from my look at the patch the overall ideas seem sound. Doing the DocumentState thing from the previous comments makes sense and will also help with this I think.

[1] https://searchfox.org/mozilla-central/rev/8f0db72fb6e35414fb9a6fc88af19c69f332425f/gfx/layers/apz/public/APZUpdater.h#165
[2] https://searchfox.org/mozilla-central/rev/8f0db72fb6e35414fb9a6fc88af19c69f332425f/gfx/layers/apz/public/APZUpdater.h#196

Comment 38

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Split out document pieces of WebRenderLayerManager r?jrmuizel

Per our discussion, this patch splits out the state management bits of
WebRenderLayerManager, allowing for them to be maintained per-document.
This was split into its own patch since *most* of the changes are
mechanical and have little to do with doc splitting itself.

Depends on D13343

Comment 39

[Alex Thayer [:alexical] (she/her)]

The (updated) full patch:

https://hg.mozilla.org/try/rev/81d461954e57948e09edbb128d95f40f70b3b643

Comment 40

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Make GpuCache document-aware r?kvark

GpuCache can currently evict things out from underneath docs which
are not updating this frame. This makes its roots document-specific,
so that we only evict items for currently updating documents.

Depends on D13343

Comment 41

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Add pref to disable texture cache clear r?bholley

To facilitate testing of document splitting before it is preffed on,
I'm adding a pref to disable clearing the texture cache, since this
will currently crash the browser with doc splitting on.

Depends on D13840

Comment 42

[Alex Thayer [:alexical] (she/her)]

Update on this now that I'm back from parental leave:

Unfortunately I didn't manage to get the mostly landable patches here landed before I left, so I suspect I'm going to have to slog through some rebasing to get them back into shape. Accordingly, my plan looks like this:

- Get all the r+ patches which don't have r- dependencies rebased and landed
- Rebase and fix all of the r- patches and get them reviewed
- Profit?

Somewhere in there I think I'm supposed to add a patch to rename all the things to whatever it is we decided to call documents. Last I heard was we're calling them pipelines, and we're renaming the existing PipelinId to DisplayListId? I think?

And since I'm alive again please feel free to ping me about anything and everything to do with document(?) splitting!

Comment 43

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ee870d4a4308
Split out document pieces of WebRenderLayerManager r=jrmuizel

Comment 44

[Oana Pop-Rus]

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

Comment 45

[Mike de Boer [:mikedeboer]]

https://hg.mozilla.org/projects/cedar/rev/ee870d4a43083769491e0964ae180304542fc10f
Bug 1441308 - Split out document pieces of WebRenderLayerManager r=jrmuizel

Comment 46

[Alex Thayer [:alexical] (she/her)]

The WebRenderLayerManager cleanup landed, and the major WR side pieces are in a PR.

As best I can tell, the remaining work here is:

• Implement a way of running a maintenance step before all frame generation in order to do document-independent texture_cache cleanup.
  • I think the best way of doing this is to add a send_transactions method to RenderApi with a list of DocumentId, Transaction pairs.
• Clean up the code to accept an arbitrary number of RenderRoots rather than hard-coding the content and chrome area RenderRoots
• Pair our usage of PipelineIds with DocumentIds (a lot of plumbing but should be fairly straightforward overall) for the APZUpdater to use as its hash map keys.
• Figure out the WebRenderScrollData situation once the previous step has been properly sorted out
• Rename all the RenderRoots and Documents to Pipeline or whatever it is we settle on

Based on what I was seeing a month or so ago, there's a good chance there will be a few minor talos regressions after all this, so those will need to be sorted out as well.

After all this, I'll probably want to focus on helping to get retained display lists enabled in the parent process.

Comment 47

[Mark]

Hi Doug, can I ask a (probably naive) question

Does there need to be a separate document for the sidebar?

IIUC one aim of splitting is to reduce power consumption by reducing the number of vibrant (transparent/blurred) pixels being sent to the OS compositor per second. To do this you'll have a vibrant but small and infrequently updating document (the toolbar) and a large and frequently updating but opaque document (the content) & each document will be sent separately to the OS compositor.

Where does the sidebar fit? It's vibrant & I think it's part of the content document. If I open a vibrant sidebar, won't the whole content document become vibrant leading to a large, frequently updating and vibrant document thus negating the savings from splitting?

Comment 48

[Alex Thayer [:alexical] (she/her)]

Since multiple documents are presently still being rendered to the same framebuffer, we're still giving the OS compositor the same number of pixels every time. Once we're sending different documents to different framebuffers I imagine this will open up different potential wins for us, but we first need to get multiple documents working with the same framebuffer.

Right now the chief goal for doc splitting is to reduce the amount of work we have to do when the chrome changes but the content does not. Content can be very simple or can be very complex - if it is very complex but relatively static, it can be very expensive right now to handle changes in the chrome area, and this can be bad because it feels like the browser itself is slow, rather than just the page you're on.

That being said, the sidebar is in a similar situation as the main toolbar, and I could certainly see it being worth it to give it its own document, but right now my only goal is to get two documents functioning properly. Once we have that, it should be much simpler to get an arbitrary number and arrangement of documents working.

Comment 49

[Mark]

Thanks very much! It sounds like we Mac users will have to wait a while longer for a battery-friendly opaque Firefox (equivalent to setting gfx.compositor.glcontext.opaque true). Cheers, Mark.

Comment 50

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/95537e83071a
Make WR caches document-aware r=bholley
https://hg.mozilla.org/integration/autoland/rev/da3629e101b9
Make GpuCache document-aware r=kvark
https://hg.mozilla.org/integration/autoland/rev/afb8c84c9677
Add pref to disable texture cache clear r=bholley

Comment 51

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/95537e83071a
https://hg.mozilla.org/mozilla-central/rev/da3629e101b9
https://hg.mozilla.org/mozilla-central/rev/afb8c84c9677

Comment 52

[Paul Rouget [:paul]]

We are trying a similar approach with Servo. Questions:

• If I understand correctly, Chrome and content use 2 different WR documents, is that right?
• Is the content document origin moved below the chrome document or the display list are shifted down? In other words, do the documents overlap?

Comment 53

[Alex Thayer [:alexical] (she/her)]

(In reply to Paul Rouget [:paul] from comment #52)

We are trying a similar approach with Servo. Questions:

• If I understand correctly, Chrome and content use 2 different WR documents, is that right?
• Is the content document origin moved below the chrome document or the display list are shifted down? In other words, do the documents overlap?

• Correct(ish). The top bar chrome gets its own document, and anything below that fold (sidebar, devtools, etc.) uses the content document. This means that content processes have no idea that there is any document splitting going on, but the parent process has to juggle two documents' display lists and other things.
• The documents do not overlap in the default setup. set_window_parameters is called with two disjoint rects for the chrome area and content area. However this is not strictly enforced, and if a user customizes their setup with weird css that keeps their URL bar at the top of the window and their tabs at the bottom or something, then we do not currently account for that which means that the content totally overlaps the chrome area.

Comment 54

[Alex Thayer [:alexical] (she/her)]

I was hoping to get patches cleaned up enough for review today, but I've run into a few things I need to debug after rebasing. However, if anyone is interested in doing a drive-by on the patches, here is the condensed form with all the patches rolled into one:

https://hg.mozilla.org/try/rev/9b372d014146ba1d4573c4600153ba246b6cc546

Comment 56

[Alex Thayer [:alexical] (she/her)]

Current status:

• It's all been split out into N render roots rather than hard-coded to two

• I've got patches functioning that make our APZ code document-aware by ensuring we have RenderRoots keying all of our hashtables and whatnot

• Ran into some difficulties in the latest rebase due to not having the clip chain ids set up on the chain of stacking context that we clone for the content render root. Kats realized though that we should be able to just accumulate the offsets of the default stacking contexts and apply that when we create our content stacking context. I just finished out the patches for this, it resolved the issue and seems to be working well

• Currently combing over all of the code that handles multiple render roots to clean it up. I originally implemented it with arrays of length wr::kRenderRootCount for all of the things we needed to split. This was fine but it's a bit ugly and difficult to read since we key them with RenderRoots which I don't want to be implicitly convertible to integers. So I added a RenderRootArray type which takes in RenderRoots as indices and I'm just bringing everything over to that

• After this I'm going to make sure I've gone through all of the review comments and then get the new patches up for review

Comment 57

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Remove unnecessary StackingContextHelper params from clips r?kats

These aren't used, so I'm just getting rid of them as cleanup.

Comment 58

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Fix bound_vao tracking in Device r?kats

Fairly self-explanatory. With multiple documents this can be a problem.

Depends on D20693

Comment 59

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Fix picture-caching interaction with doc spitting r?gw

This is a stab at what the correct approach to this should be. It
seems that we should be using the window size here and not the
screen_rect, as the screen_rect is not used to offset what we
normally draw, but instead generally for scissoring(?). The end
result is if we use an offset screen_rect, we end up applying
the offset of the chrome area twice, once because the document's
screen rect is offset, and once because of the tile.world_rect
offset.

Depends on D20696

Comment 60

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Ensure lazy init of debug renderer in draw_frame_debug_items r?gw

Document splitting is crashing with early initialization of the debug
renderer. Not sure why, and this is just a temporary workaround, but
one that I think we want anyway, as we don't want to be unnecessarily
lazy-initting the debug renderer.

Depends on D20698

Comment 61

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Core renderroot splitting changes r?kats

This just adds a few types that are commonly used by the rest of
the render root splitting patches.

Depends on D12844

Comment 62

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/deb682690025
Ensure lazy init of debug renderer in draw_frame_debug_items r=gw

Comment 63

[Natalia Csoregi [:nataliaCs]]

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

Comment 64

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Rename ForEach trait to AsyncForEach r?lsalzman

Just a little rename for clarity - I ended up scratching my head at
something for a little more time than I should have because I assumed
this was synchronous without looking at the implementation.

Depends on D20701

Comment 65

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/48bf7d6e2ea7
Fix picture-caching interaction with doc spitting r=gw

Comment 66

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/48bf7d6e2ea7

Comment 67

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Correct Renderer issues with multiple documents r?gw

This corrects A) An issue encountered with our strategy for skipping
the end_pass call for all but an offscreen render target. See the
comment above the end_pass call for details, and B) An issue with
depth clearing where we do not clear the whole rect if there are
multiple non-intersecting documents.

Depends on D20701

Comment 68

[Alex Thayer [:alexical] (she/her)]

Hey Neil, I have a question that mstange said you might be able to answer. As part of the core patch for this bug (link), I'm trying to add an attribute ("renderroot" - see the usage inside browser.xul in the patch) which when present will cause us to insert an nsDisplayRenderRoot into the display list. This is roughly similar to how nsDisplayOwnLayer works, so I cargo culted its approach and just added similar code into nsBoxFrame.cpp. However, this feels fairly hacky, and I wanted to get your take on what might be a better approach.

Ideally, to avoid future headaches, it should work on whatever XUL element we put it on. Do you have any thoughts on this? And perhaps whether we could refactor the "layer" attribute to use a similar approach?

(Apologies for the large patch - the changes in browser.xul, nsBoxFrame.cpp, and the addition of nsDisplayRenderRoot in nsDisplayList.cpp should be enough to show what the change is.)

Comment 69

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a64d942b966d
Remove unnecessary StackingContextHelper params from clips r=kats

Comment 70

[Neil Deakin]

(In reply to Doug Thayer [:dthayer] from comment #68)

Hey Neil, I have a question that mstange said you might be able to answer. As part of the core patch for this bug (link), I'm trying to add an attribute ("renderroot" - see the usage inside browser.xul in the patch) which when present will cause us to insert an nsDisplayRenderRoot into the display list. This is roughly similar to how nsDisplayOwnLayer works, so I cargo culted its approach and just added similar code into nsBoxFrame.cpp. However, this feels fairly hacky, and I wanted to get your take on what might be a better approach.

That's probably ok for now. Note that nsBoxFrame is used for anything that has CSS display: -moz-box (and related types that inherit it) and isn't specifically a XUL element. Some XUL elements that don't have children such as images and labels use nsLeafBoxFrame that does not inherit from nsBoxFrame. The patch here also adds renderroot="content" to some html:div elements in browser.xul but I don't see where that is handled.

If you want it to be supported on all elements, you might need to place the support somewhere in the display list handling (I'm not familiar enough with display lists to know what this would mean). I'm guessing that you probably want to only support this on specific container type elements, so in the long run you might want to create or require a specific element that does this, and wrap existing content inside it.

Comment 71

[Natalia Csoregi [:nataliaCs]]

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

Comment 72

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Map document layers to z ranges r?gw

As discussed in IRC. AFAICT the ORTHO_NEAR|FAR_PLANE should match
up with the ranges of valid ZBufferIds, but please double-check
me.

Depends on D23056

Comment 73

[Alex Thayer [:alexical] (she/her)]

Try runs:

Pref off: https://treeherder.mozilla.org/#/jobs?repo=try&revision=41e9fb24c1e82c73517d7ab724f53e98dcc35cfe
Pref on: https://treeherder.mozilla.org/#/jobs?repo=try&revision=485638ad90544da7c6c5f324378b4d524370c2c5

(They're still building at the time of me posting this. They were green before rebasing and adding the new changesets.)

Comment 74

[Alex Thayer [:alexical] (she/her)]

Try did not seem to like that. Investigating shortly.

Comment 75

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Support moving Maybe's in IPC serialization r?mccr8

I'd like to move an ipc::ByteBuf member of a struct into a Maybe,
and in order for that to work IPDLParamTraits<Maybe> needs to support
the rvalue Write.

Depends on D23599

Comment 76

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1441308 - Always send parent commands when sending mDestroyedActors r?kats

If we try to send them separately as we were before, we can run into
cases where we try to destroy the actors and then send the OpRemoveTexture,
which crashes.

Depends on D23986

Comment 77

[Alex Thayer [:alexical] (she/her)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b2342bfab4151003085eb871d10bc1102e45bed

Looks green to me? Let me know if it looks like I mis-classified anything.

Try with pref off (EDIT: on) is running here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=557dda58ad01f526ef29619f8578438952d11e32

Comment 78

[Kartikaya Gupta (email:[e-mail])]

(In reply to Doug Thayer [:dthayer] from comment #77)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b2342bfab4151003085eb871d10bc1102e45bed

Looks green to me? Let me know if it looks like I mis-classified anything.

This looks ok to me.

Try with pref off is running here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=557dda58ad01f526ef29619f8578438952d11e32

This is with pref on and it's not looking so good, but I'm ok with landing it with the pref off and then add additional patches to clean this up.

Comment 79

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/737807563dd5
Add the render root splitting pref r=sotaro
https://hg.mozilla.org/integration/autoland/rev/c09a51622e98
Core renderroot splitting changes r=kats,sotaro
https://hg.mozilla.org/integration/autoland/rev/baee8ada680f
Rename ForEach trait to AsyncForEach r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/e2f83e4816dd
Correct Renderer issues with multiple documents r=gw
https://hg.mozilla.org/integration/autoland/rev/d8b4d6ec9b40
Map document layers to z ranges r=gw
https://hg.mozilla.org/integration/autoland/rev/32f7793dfd1a
Support moving Maybe's in IPC serialization r=mccr8
https://hg.mozilla.org/integration/autoland/rev/1764701d11d1
Always send parent commands when sending mDestroyedActors r=kats,sotaro

Comment 80

[Andrei Ciure[:aciure]]

Backed out 7 changesets (bug 1441308)for causing webrender build bustages

push that caused the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=235328305&revision=1764701d11d103a2ac02f8cfa5ff4511b8f5dd70

failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&fromchange=d66a126be7b677ef148dd30f20371a212fece804&selectedJob=235379756&searchStr=linux%2Cx64%2Cquantumrender%2Cdebug%2Cwebrender%2Cstandalone%2Cwebrender-linux-debug%2Cwr%28wrench%29

backout: https://hg.mozilla.org/integration/autoland/rev/699a6b6bee44710ba8710a2ee7740783a596a26a

Comment 81

[Alex Thayer [:alexical] (she/her)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=800bc0452ccfb193493522dfc0a5160fdab12720

Going for it again...

Comment 82

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8cefa694f811
Add the render root splitting pref r=sotaro
https://hg.mozilla.org/integration/autoland/rev/96da9d241051
Core renderroot splitting changes r=kats,sotaro
https://hg.mozilla.org/integration/autoland/rev/e36670b15dd6
Rename ForEach trait to AsyncForEach r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/31eb0acf3a1d
Correct Renderer issues with multiple documents r=gw
https://hg.mozilla.org/integration/autoland/rev/1d405e872db4
Map document layers to z ranges r=gw
https://hg.mozilla.org/integration/autoland/rev/8761ce294d2c
Support moving Maybe's in IPC serialization r=mccr8
https://hg.mozilla.org/integration/autoland/rev/9bfd4e60ec4e
Always send parent commands when sending mDestroyedActors r=kats,sotaro

Comment 83

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/8cefa694f811
https://hg.mozilla.org/mozilla-central/rev/96da9d241051
https://hg.mozilla.org/mozilla-central/rev/e36670b15dd6
https://hg.mozilla.org/mozilla-central/rev/31eb0acf3a1d
https://hg.mozilla.org/mozilla-central/rev/1d405e872db4
https://hg.mozilla.org/mozilla-central/rev/8761ce294d2c
https://hg.mozilla.org/mozilla-central/rev/9bfd4e60ec4e

Comment 84

[Sotaro Ikeda [:sotaro]]

I run MotionMark test with gfx.webrender.split-render-roots:true. Sometimes rendering became all white or all black.

Comment 85

[Peter Van der Beken [:peterv]]

https://hg.mozilla.org/projects/ash/rev/8cefa694f81188548a52bce32825ecfbd7e3c3c6
Bug 1441308 - Add the render root splitting pref r=sotaro

https://hg.mozilla.org/projects/ash/rev/96da9d241051d222bdd693bdb519d058a013b59c
Bug 1441308 - Core renderroot splitting changes r=kats,sotaro

https://hg.mozilla.org/projects/ash/rev/e36670b15dd6852cf3695420f2e051bd0fa945cd
Bug 1441308 - Rename ForEach trait to AsyncForEach r=lsalzman

https://hg.mozilla.org/projects/ash/rev/31eb0acf3a1dd86cbbb21315d32826241e29e227
Bug 1441308 - Correct Renderer issues with multiple documents r=gw

https://hg.mozilla.org/projects/ash/rev/1d405e872db459002abc43d312ffe8c240e1f83f
Bug 1441308 - Map document layers to z ranges r=gw

https://hg.mozilla.org/projects/ash/rev/8761ce294d2c199f454ceb9bd5907296ba5e62bc
Bug 1441308 - Support moving Maybe's in IPC serialization r=mccr8

https://hg.mozilla.org/projects/ash/rev/9bfd4e60ec4e36b680c1722a3bd3e4b7359f7e36
Bug 1441308 - Always send parent commands when sending mDestroyedActors r=kats,sotaro

Comment 86

[Alex Thayer [:alexical] (she/her)]

Summary of the current state of document splitting (with a view to my involvement with it and where that involvement should end):

Bug 1538540 - Crash in [@ core::option::expect_failed | webrender::renderer::TextureResolver::bind]

This seems to have died down close to nothing. I suspect that the remaining crashes fall into two categories. 1) Crashes that would be happening without the patches from this bug. 2) Crashes that only happen with the document splitting pref on. Regarding 1), I think that's clearly outside the scope of my involvement. Regarding 2), I don't have any specific leads as to what might be causing this. I've been looking through the source and I think it's going to take a deeper dive which someone more familiar with WebRender itself might be able to do more quickly. However I'd be happy to be a sounding board for their ideas, and if absolutely necessary I can tackle this, I just suspect I wouldn't be the most efficient choice.

Bug 1547351 - Contents on popup of Addons are shown as white when gfx.webrender.split-render-roots is true

I fixed this on OSX it seems, but the patch appears to not fix things for Windows (uncertain about Linux - will investigate). I will continue working on the remaining bits of this for non-OSX.

Bug 1548247 - document splitting: Sluggish autoscrolling

I have a patch which fixes the exact issue mentioned in the bug, but it seems there are still scrolling problems on about:support and other parent process pages which I haven't been able to sort out. I've been scratching my head against them but I think it will be best if kats takes the rest. I will put up my patches for fixing the autoscrolling issue for review by kats and then I think it will be most efficient if he can take it over.

Bug 1548688 - document splitting: Picture-in-Picture player buttons are visible, but the video itself is just white

I will take this one (unless it unearths some bigger problem that I don't know about. I think this is unlikely).

Bug 1549976 - document splitting: "minecraft.net has control of your mouse pointer" slips behind website content

I think this is going to be rather large. I have a sketch in mind of what we'll need to do, but I think it's going to be a fair bit of code. My proposal for this is that I write up patches to get this mostly working, and that someone more familiar with the gfx pipeline takes that over and hammers out the kinks. I am open to pushback on this, though. I think it's anyone's guess as far as whether I would be the most efficient person for this work after identifying the major pieces.

jbonisteel / pslawless, what are your thoughts? I think these bugs can be in good shape for 59. However, it's not certain that this encompasses all of the work needed to turn on document splitting on Nightly, and it's quite likely that there will be more work which will only surface when we do turn this on for Nightly, so we should probably articulate a plan for my involvement with those unknowns, yes?

Comment 87

[Jessie [:jbonisteel] pls NI]

Thanks Doug - that sounds good. For 1538540 we will leave this for now as it has died down and if it picks back up when we re-enable, we will figure out what to do. For 1549976 can you ping me when you have the patches up for the mostly working stuff? That will help determine who on GFX should help with the rest.

Comment 88

[Sotaro Ikeda [:sotaro]]

With latest m-c, enabling document split with pref gfx.webrender.split-render-roots:true caused almost all test failures :(
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8360c69097cd7fa5e1588cb9131e9d38c3aa2e62