Bug 207154 – REGRESSION (r254428): [ macOS wk1 ] inspector/canvas/recording-2d-full.html became slower, flakily times out

• Status: RESOLVED
• Resolution: FIXED
• Component: Canvas

[Jacob Uphoff]

inspector/canvas/recording-2d-full.html

This test has been a flaky timeout for all visible history on wk1 debug 

I was not able to reproduce issue with iteration runs

History:

https://results.webkit.org/?suite=layout-tests&test=inspector%2Fcanvas%2Frecording-2d-full.html&limit=50000&platform=ios&platform=mac

Diff:

--- /Volumes/Data/slave/catalina-debug-tests-wk1/build/layout-test-results/inspector/canvas/recording-2d-full-expected.txt
+++ /Volumes/Data/slave/catalina-debug-tests-wk1/build/layout-test-results/inspector/canvas/recording-2d-full-actual.txt
@@ -1,3 +1,4 @@
+FAIL: Timed out waiting for notifyDone to be called
 Test that CanvasManager is able to record actions made to 2D canvas contexts.
 
 
@@ -1013,19 +1014,4 @@
       trace:
         0: (anonymous function)
         1: executeFrameFunction
-    1: width = 2
-      swizzleTypes: [Number]
-      trace:
-        0: (anonymous function)
-        1: executeFrameFunction
-  74: (duration)
-    0: height
-      trace:
-        0: (anonymous function)
-        1: executeFrameFunction
-    1: height = 2
-      swizzleTypes: [Number]
-      trace:
-        0: (anonymous function)
-        1: executeFrameFunction

[Radar WebKit Bug Importer]

<rdar://problem/59127935>

[Jacob Uphoff]

Created attachment 389580 [details]
Update Test Expectations

[Ryan Haddad]

Comment on attachment 389580 [details]
Update Test Expectations

Clearing flags on attachment: 389580

Committed r255607: <https://trac.webkit.org/changeset/255607>

[Alexey Proskuryakov]

> This test has been a flaky timeout for all visible history on wk1 debug 

You may be referring to internal test results, where this test was failing on slower bots. But there is actually a sharp start around r254428, where LFC integration patches landed.

Enabling test times on the flakiness dashboard, I see a clear performance regression. It's not visible on release bots, but it's very clear on debug, ASan and GuardMalloc. I think that it's not just more complex assertions, but more memory allocations, which is why GuardMalloc suffered more.

[Alexey Proskuryakov]

*** Bug 207145 has been marked as a duplicate of this bug. ***

[Devin Rousso]

Created attachment 389860 [details]
Patch

[zalan]

you may wanna undo r255607. It's not slow anymore.

[Alexey Proskuryakov]

And r255591 too.

[Devin Rousso]

Comment on attachment 389860 [details]
Patch

(In reply to zalan from comment #7)
> you may wanna undo r255607. It's not slow anymore.
(In reply to Alexey Proskuryakov from comment #8)
> And r255591 too.
I think we still want these tests to be marked as slow, as they can sometimes take a while, especially in debug builds.  The payload being sent to Web Inspector is very large (it has base64 dataURLs in it), as well as heavy use of async/await.

[zalan]

(In reply to Devin Rousso from comment #9)
> Comment on attachment 389860 [details]
> Patch
> 
> (In reply to zalan from comment #7)
> > you may wanna undo r255607. It's not slow anymore.
> (In reply to Alexey Proskuryakov from comment #8)
> > And r255591 too.
> I think we still want these tests to be marked as slow, as they can
> sometimes take a while, especially in debug builds.  The payload being sent
> to Web Inspector is very large (it has base64 dataURLs in it), as well as
> heavy use of async/await.
Could we test the bots first and make the decision based on that?

[WebKit Commit Bot]

Comment on attachment 389860 [details]
Patch

Clearing flags on attachment: 389860

Committed r255895: <https://trac.webkit.org/changeset/255895>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.