Summary: | fast/dom/inline-event-attributes-release.html randomly fails | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Anders Carlsson <andersca> | ||||||||||
Component: | DOM | Assignee: | Fujii Hironori <Hironori.Fujii> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | Hironori.Fujii, rniwa, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar, MakingBotsRed | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Anders Carlsson
2012-04-10 14:28:30 PDT
Another test failure: Buildbot: builder Apple-BigSur-Release-WK1-Tests build 11350: 251845@main https://build.webkit.org/#/builders/48/builds/11350 --- /Volumes/Data/worker/Apple-BigSur-Release-WK1-Tests/build/layout-test-results/fast/dom/inline-event-attributes-release-expected.txt +++ /Volumes/Data/worker/Apple-BigSur-Release-WK1-Tests/build/layout-test-results/fast/dom/inline-event-attributes-release-actual.txt @@ -3,7 +3,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". -PASS afterCount - beforeCount is 0 +FAIL afterCount - beforeCount should be 0. Was 1. PASS successfullyParsed is true TEST COMPLETE Another test failure: Buildbot: builder Apple-Monterey-Release-WK2-Tests build 4424 251329@main https://build.webkit.org/#/builders/365/builds/4424 --- /Volumes/Data/worker/Apple-Monterey-Release-WK2-Tests/build/layout-test-results/fast/dom/inline-event-attributes-release-expected.txt +++ /Volumes/Data/worker/Apple-Monterey-Release-WK2-Tests/build/layout-test-results/fast/dom/inline-event-attributes-release-actual.txt @@ -3,7 +3,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". -PASS afterCount - beforeCount is 0 +FAIL afterCount - beforeCount should be 0. Was 1. PASS successfullyParsed is true TEST COMPLETE Created attachment 460694 [details]
Patch
Another test failure: Buildbot: builder Apple-BigSur-Release-WK1-Tests build 10954: 251198@main https://build.webkit.org/#/builders/48/builds/10954 --- /Volumes/Data/worker/Apple-BigSur-Release-WK1-Tests/build/layout-test-results/fast/dom/inline-event-attributes-release-expected.txt +++ /Volumes/Data/worker/Apple-BigSur-Release-WK1-Tests/build/layout-test-results/fast/dom/inline-event-attributes-release-actual.txt @@ -3,7 +3,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". -PASS afterCount - beforeCount is 0 +FAIL afterCount - beforeCount should be 0. Was -110. PASS successfullyParsed is true TEST COMPLETE Comment on attachment 460694 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=460694&action=review > LayoutTests/fast/dom/inline-event-attributes-release.html:27 > +if (delta < iterations / 10) This is a very high factor. I suggest we use 2 or 3 instead. Comment on attachment 460694 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=460694&action=review >> LayoutTests/fast/dom/inline-event-attributes-release.html:27 >> +if (delta < iterations / 10) > > This is a very high factor. I suggest we use 2 or 3 instead. I retrieved the number from checkForNodeLeaks. https://github.com/WebKit/WebKit/blob/e36b05647cc7b60f59a9c96872d4f562c9514e7c/LayoutTests/fast/dom/reference-cycle-leaks.html#L21 And, testDocumentIsNotLeaked passes if at least a single document is reclaimed. https://github.com/WebKit/WebKit/blob/e36b05647cc7b60f59a9c96872d4f562c9514e7c/LayoutTests/resources/gc.js#L49 However, I agree with you. Will fix this test. Created attachment 460696 [details]
Patch
Comment on attachment 460696 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=460696&action=review > LayoutTests/fast/dom/inline-event-attributes-release.html:27 > +if (delta < 3) Oh, no, I meant to say divide iterations by 3, not literal 3. Requiring at least one document getting collected is okay too. Delta to be less than 10 is too aggressive of a policy. Comment on attachment 460696 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=460696&action=review >> LayoutTests/fast/dom/inline-event-attributes-release.html:27 >> +if (delta < 3) > > Oh, no, I meant to say divide iterations by 3, not literal 3. > Requiring at least one document getting collected is okay too. > Delta to be less than 10 is too aggressive of a policy. Oh. Will fix. Created attachment 460701 [details]
Patch
Comment on attachment 460701 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=460701&action=review > LayoutTests/fast/dom/inline-event-attributes-release.html:26 > +if (delta < iterations / 3) iterations / 2 might be better and this might be still flaky but we can try. Comment on attachment 460701 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=460701&action=review >> LayoutTests/fast/dom/inline-event-attributes-release.html:26 >> +if (delta < iterations / 3) > > iterations / 2 might be better and this might be still flaky but we can try. Why do you think it might be still flaky? fast/dom/reference-cycle-leaks.html is using the condition "if (leaks < repetitions / 10)", but not flaky. (In reply to Fujii Hironori from comment #12) > Comment on attachment 460701 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=460701&action=review > > >> LayoutTests/fast/dom/inline-event-attributes-release.html:26 > >> +if (delta < iterations / 3) > > > > iterations / 2 might be better and this might be still flaky but we can try. > > Why do you think it might be still flaky? > fast/dom/reference-cycle-leaks.html is using the condition "if (leaks < > repetitions / 10)", but not flaky. Because it's conservative GC. The assertion that more than half the nodes get collected is only probabilistically true. Created attachment 460705 [details]
Patch for landing
I don't think the conservative GC can make so much false-positive in this test case. However, it would happen in some cases, for example, if all created element objects were pushed into an array, and the array wasn't zero-filled after used. Committed 252173@main (45cf55da6e5b): <https://commits.webkit.org/252173@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 460705 [details]. |