Summary: | Be more conservative with compositing layer creation when memory is low | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||||||
Component: | New Bugs | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | dino, ews-watchlist, koivisto, mjs, realdawei, rniwa, ryanhaddad, saam, simon.fraser, thorton, tsavell, zalan | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=188357 https://bugs.webkit.org/show_bug.cgi?id=188787 |
||||||||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2018-07-20 11:07:50 PDT
Created attachment 345464 [details]
WIP Patch
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. 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
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. 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
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. 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
Created attachment 345821 [details]
Patch
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? (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. (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(. (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 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? + 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(); + } I'm looking at the WK1 failures. Followup in bug 188138. 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 I will look (these may be expected). Do we have any timeframe on when these tests will be fixed or an update on what's happening? 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. 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. |