Bug 213373

Summary: Implement serial jsc stress tests
Product: WebKit Reporter: Angelos Oikonomopoulos <angelos>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: clopez, guijemont, jsc32, keith_miller, mark.lam, pmatos, saam, ticaiolima, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 210790    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Angelos Oikonomopoulos
Reported 2020-06-19 06:45:44 PDT
Implement serial jsc stress tests
Attachments
Patch (5.75 KB, patch)
2020-06-19 06:50 PDT, Angelos Oikonomopoulos 		no flags
DetailsFormatted DiffDiff
Patch (6.33 KB, patch)
2020-06-21 07:59 PDT, Angelos Oikonomopoulos 		no flags
DetailsFormatted DiffDiff
Patch (6.34 KB, patch)
2020-07-09 23:39 PDT, Angelos Oikonomopoulos 		no flags
DetailsFormatted DiffDiff
Patch (6.34 KB, patch)
2020-07-10 01:40 PDT, Angelos Oikonomopoulos 		no flags
DetailsFormatted DiffDiff
Patch (6.38 KB, patch)
2020-07-10 07:27 PDT, Angelos Oikonomopoulos 		no flags
DetailsFormatted DiffDiff
Patch (6.38 KB, patch)
2020-07-12 01:56 PDT, Angelos Oikonomopoulos 		no flags
DetailsFormatted DiffDiff
Patch (8.65 KB, patch)
2020-12-15 06:46 PST, Angelos Oikonomopoulos 		no flags
DetailsFormatted DiffDiff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.
Angelos Oikonomopoulos
Comment 1 2020-06-19 06:50:07 PDT
Created attachment 402277 [details] Patch
Angelos Oikonomopoulos
Comment 2 2020-06-19 07:05:44 PDT
See also https://bugs.webkit.org/show_bug.cgi?id=210790
Guillaume Emont
Comment 3 2020-06-19 08:09:02 PDT
Should this be applying only when $memoryLimited is true? Can your current option be used in that way? e.g. //@ serial! if $memoryLimited
Guillaume Emont
Comment 4 2020-06-19 08:10:53 PDT
Also, I think stress/check-is-constant-non-cell-should-not-array-profile-during-osr-exit.js could benefit from this treatment, unless it can be rewritten to take less memory (at least on an rpi3, it seems that it can run in serial). See Bug 213209.
Angelos Oikonomopoulos
Comment 5 2020-06-21 07:59:25 PDT
Created attachment 402423 [details] Patch
Angelos Oikonomopoulos
Comment 6 2020-06-21 08:00:32 PDT
(In reply to Guillaume Emont from comment #3) > Should this be applying only when $memoryLimited is true? Can your current > option be used in that way? e.g. > > //@ serial! if $memoryLimited Ugh, yah, of course. That was the intention, thanks for catching this.
Angelos Oikonomopoulos
Comment 7 2020-06-21 08:03:38 PDT
(In reply to Guillaume Emont from comment #4) > Also, I think > stress/check-is-constant-non-cell-should-not-array-profile-during-osr-exit. > js could benefit from this treatment, unless it can be rewritten to take > less memory (at least on an rpi3, it seems that it can run in serial). See > Bug 213209. Right. Including check-is-constant-non-cell-should-not-array-profile-during-osr-exit.js and marking both tests with //@ serial! if $memoryLimited in the new patch.
Paulo Matos
Comment 8 2020-07-06 03:24:19 PDT
This is a really useful patch, thanks @angelos. I am retrying builds on ews to see if all is good.
Caio Lima
Comment 9 2020-07-07 03:34:54 PDT
Comment on attachment 402423 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402423&action=review > JSTests/stress/check-is-constant-non-cell-should-not-array-profile-during-osr-exit.js:1 > +//@ serial! if $memoryLimited I think we would like to only serialize this test on ARMv7 or MIPS. Right now it is still crashing on ARMv7, according EWS
Caio Lima
Comment 10 2020-07-09 10:47:47 PDT
Comment on attachment 402423 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402423&action=review >> JSTests/stress/check-is-constant-non-cell-should-not-array-profile-during-osr-exit.js:1 >> +//@ serial! if $memoryLimited > > I think we would like to only serialize this test on ARMv7 or MIPS. Right now it is still crashing on ARMv7, according EWS The issue with ARMv7 is that this test is being killed by memory monitor. I think we can handle this issue when we properly configure `JSCTEST_memoryLimit` into our bots.
Angelos Oikonomopoulos
Comment 11 2020-07-09 23:39:48 PDT
Created attachment 403946 [details] Patch
Angelos Oikonomopoulos
Comment 12 2020-07-10 01:40:55 PDT
Created attachment 403952 [details] Patch
Angelos Oikonomopoulos
Comment 13 2020-07-10 07:27:29 PDT
Created attachment 403963 [details] Patch
Angelos Oikonomopoulos
Comment 14 2020-07-12 01:56:53 PDT
Created attachment 404091 [details] Patch
Yusuke Suzuki
Comment 15 2020-12-04 18:42:36 PST
Comment on attachment 404091 [details] Patch The idea is awesome!! Can you create ChangeLog by uploading the patch via `webkit-patch upload --update-changelogs`? And can you describe what this patch does by how in ChangeLog?
Angelos Oikonomopoulos
Comment 16 2020-12-07 04:15:16 PST
(In reply to Yusuke Suzuki from comment #15) > Comment on attachment 404091 [details] > Patch > > The idea is awesome!! > Can you create ChangeLog by uploading the patch via `webkit-patch upload > --update-changelogs`? And can you describe what this patch does by how in > ChangeLog? Sure, I can do that :-) I'll try to test the patch locally first though. One consideration is that serial! will be completely ignored by the new --gnu-parallel-runner; but perhaps this can go in regardless and updating --gnu-parallel-runner to honor serial! can be a separate patch. Also, since we're setting --retries for the parallel runner, a memory-hungry test is likely to pass eventually anyway, since the chances are it will get rescheduled until it's eventually the only "large" test running on the memory-limited device.
Angelos Oikonomopoulos
Comment 17 2020-12-15 06:46:30 PST
Created attachment 416241 [details] Patch
Angelos Oikonomopoulos
Comment 18 2020-12-15 06:48:58 PST
Updated the patch to also work with --gnu-parallel-runner and to not introduce serial! for any tests yet. Would rather do that for individual tests along with bumping the current default limit in buildbots where the remote boards have this much RAM, otherwise serial tests will still run into the default JSCTEST_memoryLimit of 600MB.
Yusuke Suzuki
Comment 19 2021-01-05 15:05:06 PST
Comment on attachment 416241 [details] Patch r=me, can you file a bug doing the same thing for `jsc-stress-test-writer-ruby.rb`?
EWS
Comment 20 2021-01-05 15:14:46 PST
Committed r271179: <https://trac.webkit.org/changeset/271179> All reviewed patches have been landed. Closing bug and clearing flags on attachment 416241 [details].
Radar WebKit Bug Importer
Comment 21 2021-01-05 15:15:16 PST
<rdar://problem/72831399>
Note You need to log in before you can comment on or make changes to this bug.