Bug 187866 – Be more conservative with compositing layer creation when memory is low

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Simon Fraser (smfr)]

Be more conservative with compositing layer creation when memory is low

[Simon Fraser (smfr)]

Created attachment 345464 [details]
WIP Patch

[Simon Fraser (smfr)]

rdar://problem/42366345

[EWS Watchlist]

Comment on attachment 345464 [details]
WIP Patch

Attachment 345464 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8601523

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 345468 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 345464 [details]
WIP Patch

Attachment 345464 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8601525

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 345469 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 345464 [details]
WIP Patch

Attachment 345464 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/8601454

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 345471 [details]
Archive of layout-test-results from ews113 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Simon Fraser (smfr)]

Created attachment 345821 [details]
Patch

[Dean Jackson]

Comment on attachment 345821 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=345821&action=review

> Source/WebCore/platform/graphics/transforms/Matrix3DTransformOperation.h:51
> +    bool isRepresentableIn2D() const final;

why not inline?

[Simon Fraser (smfr)]

(In reply to Dean Jackson from comment #10)
> Comment on attachment 345821 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=345821&action=review
> 
> > Source/WebCore/platform/graphics/transforms/Matrix3DTransformOperation.h:51
> > +    bool isRepresentableIn2D() const final;
> 
> why not inline?

Virtual functions are never inlined by the compiler.

[Maciej Stachowiak]

(In reply to Simon Fraser (smfr) from comment #11)
>
> Virtual functions are never inlined by the compiler.

They can be inlined if the compiler can statically prove the exact type, which sometimes it can (not sure if that applies in this case(.

[Saam Barati]

(In reply to Maciej Stachowiak from comment #12)
> (In reply to Simon Fraser (smfr) from comment #11)
> >
> > Virtual functions are never inlined by the compiler.
> 
> They can be inlined if the compiler can statically prove the exact type,
> which sometimes it can (not sure if that applies in this case(.

Right. I think a common case where this happens a lot is when a class is marked as final

[alan baradlay]

Comment on attachment 345821 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=345821&action=review

> Source/WebCore/rendering/RenderLayerCompositor.cpp:2458
> +    if (!renderer.isCanvas())

is<RenderHTMLCanvas>

> Source/WebCore/rendering/RenderLayerCompositor.cpp:2461
> +    bool isCanvasLargeEnoughToForceCompositing = true;

auto?

[Simon Fraser (smfr)]

+    switch (m_compositingPolicy) {
+    case CompositingPolicy::Normal:
+        return renderer.style().transform().has3DOperation();
+    case CompositingPolicy::Conservative:
+        // Continue to allow pages to avoid the very slow software filter path.
+        if (renderer.style().transform().has3DOperation() && renderer.hasFilter())
+            return true;
+        return !renderer.style().transform().isRepresentableIn2D();
+    }

[Simon Fraser (smfr)]

https://trac.webkit.org/changeset/234330/webkit

[Simon Fraser (smfr)]

I'm looking at the WK1 failures.

[Simon Fraser (smfr)]

Followup in bug 188138.

[Truitt Savell]

It looks like there are three tests on High Sierra WK1 that are continuing to fail consistently after https://trac.webkit.org/changeset/234330/webkit. The tests are flakey on other WK1 release platforms.

Tests:
legacy-animation-engine/compositing/backing/transform-transition-from-outside-view.html 
legacy-animation-engine/compositing/geometry/limit-layer-bounds-opacity-transition.html 
legacy-animation-engine/compositing/layer-creation/animation-overlap-with-children.html

Test History
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=legacy-animation-engine%2Fcompositing%2Fbacking%2Ftransform-transition-from-outside-view.html%20legacy-animation-engine%2Fcompositing%2Fgeometry%2Flimit-layer-bounds-opacity-transition.html%20legacy-animation-engine%2Fcompositing%2Flayer-creation%2Fanimation-overlap-with-children.html

Text Diffs:
https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK1%20(Tests)/r234363%20(7001)/legacy-animation-engine/compositing/backing/transform-transition-from-outside-view-diff.txt

https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK1%20(Tests)/r234363%20(7001)/legacy-animation-engine/compositing/geometry/limit-layer-bounds-opacity-transition-diff.txt

https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK1%20(Tests)/r234363%20(7001)/legacy-animation-engine/compositing/layer-creation/animation-overlap-with-children-diff.txt

[Simon Fraser (smfr)]

I will look (these may be expected).

[Truitt Savell]

Do we have any timeframe on when these tests will be fixed or an update on what's happening?

[Antti Koivisto]

Comment on attachment 345821 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=345821&action=review

> Source/WebCore/page/Page.h:159
> +    Conservative, // Used in low memory situations.

"Conservative" is rather vague. How about just calling this CompositingPolicy::LowMemory? You can remove the comment too.

[Truitt Savell]

I found that r234330 has also caused two tests to fail together:
inspector/layers/layer-tree-manager.html
inspector/layers/layers-anonymous.html

Test history:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector%2Flayers%2Flayer-tree-manager.html%20inspector%2Flayers%2Flayers-anonymous.html

These two tests will fail together often on High Sierra Leaks and sometimes on other WK1 builds. This is all tracked in https://bugs.webkit.org/show_bug.cgi?id=188394

I was able to find a reliable reproduction method and it looks like https://trac.webkit.org/changeset/234330/webkit is at fault.