Bug 215353 - [ macOS iOS ] svg/animations/smil-leak-element-instances-noBaseValRef.svg is a flaky failure
Summary: [ macOS iOS ] svg/animations/smil-leak-element-instances-noBaseValRef.svg is ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-08-10 18:17 PDT by Hector Lopez
Modified: 2020-09-08 17:52 PDT (History)
7 users (show)

See Also:


Attachments
Patch (3.68 KB, patch)
2020-08-13 01:23 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (3.79 KB, patch)
2020-08-17 15:33 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (2.99 KB, patch)
2020-09-08 11:26 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hector Lopez 2020-08-10 18:17:34 PDT
svg/animations/smil-leak-element-instances-noBaseValRef.svg

Test is a flaky failure on macOS wk 2 Release and iOS wk2 Debug according to history. First occurrence of failure on macOS appears at r254248. First occurrence of failure on iOS appears at r259172.

History:
https://results.webkit.org/?suite=layout-tests&test=svg%2Fanimations%2Fsmil-leak-element-instances-noBaseValRef.svg&platform=ios&platform=mac&limit=50000

Diff:
--- /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/svg/animations/smil-leak-element-instances-noBaseValRef-expected.txt
+++ /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/svg/animations/smil-leak-element-instances-noBaseValRef-actual.txt
@@ -1 +1 @@
-PASS
+FAIL: 3 extra live node(s)
Comment 1 Radar WebKit Bug Importer 2020-08-10 18:17:59 PDT
<rdar://problem/66810097>
Comment 2 Hector Lopez 2020-08-10 18:27:52 PDT
Test expectations while investigated:

https://trac.webkit.org/changeset/265484/webkit
Comment 3 Said Abou-Hallawa 2020-08-12 15:36:09 PDT
Here is the history of this test:

r115518     Added smil-leak* tests
r155648     Changed the smil-leak* tests by moving garbage collection to the next run-loop after touching the nodes
r155882     Changed the smil-leak* tests by trying garbage collection mutiple times
r155890     Changed the smil-leak* tests by changing settimeout(, 0) to settimeout(, 100)
r155897     Changed platform/mac/TestExpectations - marked smil-leak* tests flaky on mac ports only
r181345     Caused smil-leak* tests to be more flaky
r181401     Changed platform/mac/TestExpectations- unmarked smil-leak* tests flaky on mac ports
r181401     Changed TestExpectations - marked smil-leak* tests flaky on all ports
r196268     Changed TestExpectations - unmarked smil-leak* tests un-flaky on all ports
r226187     Changed platform/mac-wk1/TestExpectations - marked smil-leak-element-instances-noBaseValRef.svg flaky on mac-wk1
r234441     Changed platform/mac-wk1/TestExpectations - unmarked smil-leak-element-instances-noBaseValRef.svg un-flaky on mac-wk1
r265484     Changed platform/ios-wk2/TestExpectations - marked smil-leak-element-instances-noBaseValRef.svg falky on ios-wk2
r265484     Changed platform/mac-wk2/TestExpectations - marked smil-leak-element-instances-noBaseValRef.svg falky on mac-wk2
Comment 4 Said Abou-Hallawa 2020-08-13 01:00:37 PDT
From the history of this test and from other tests, I think the flakiness is in GCController.collect(). Calling this function from JS does not guarantee the memory of all the deallocated objects is garbage-collected. Multiple calls to GCController.collect() may be invoked to overcome this problem. Putting idle time before requesting the numberOfLiveNodes() might be need as well.

This is a stress test. It adds 100 <use> elements. In each one of them an <animate> element is added. So these objects are allocated then deallocated:

1. <use> elements, each with animated properties.
2. Shadow trees in which there is a <rect> element is created with its animated properties 
3. <animate> elements
4. SVGAnimators for the <animate> elements
5. Renderers for all the <use> and the <rect> elements in the shadow trees.

Since the GCController.collect() is flaky, I think we have to make the number of elements small such that there is kind of guarantee all the deallocated memory will be garbage collected during the test trials. I think adding three <use> elements should be enough to test the SVG animated properties do not leak. And allocating and freeing this number of objects should not exercise the flakiness of GCController.collect().
Comment 5 Said Abou-Hallawa 2020-08-13 01:23:45 PDT
Created attachment 406502 [details]
Patch
Comment 6 EWS 2020-08-17 15:05:22 PDT
Tools/Scripts/svn-apply failed to apply attachment 406502 [details] to trunk.
Please resolve the conflicts and upload a new patch.
Comment 7 Said Abou-Hallawa 2020-08-17 15:33:36 PDT
Created attachment 406749 [details]
Patch
Comment 8 EWS 2020-08-17 16:13:03 PDT
Committed r265780: <https://trac.webkit.org/changeset/265780>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 406749 [details].
Comment 9 Said Abou-Hallawa 2020-09-08 11:26:09 PDT
Reopening to attach new patch.
Comment 10 Said Abou-Hallawa 2020-09-08 11:26:09 PDT
Created attachment 408244 [details]
Patch
Comment 11 Said Abou-Hallawa 2020-09-08 11:29:23 PDT
After r265780, bots still show flakiness. Although the flakiness became very rare but it still exists.
Comment 12 Geoffrey Garen 2020-09-08 14:25:47 PDT
Comment on attachment 408244 [details]
Patch

r=me
Comment 13 EWS 2020-09-08 17:52:58 PDT
Committed r266765: <https://trac.webkit.org/changeset/266765>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 408244 [details].