Bug 213373 – Implement serial jsc stress tests

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Angelos Oikonomopoulos]

Implement serial jsc stress tests

[Angelos Oikonomopoulos]

Created attachment 402277 [details]
Patch

[Angelos Oikonomopoulos]

See also https://bugs.webkit.org/show_bug.cgi?id=210790

[Guillaume Emont]

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]

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]

Created attachment 402423 [details]
Patch

[Angelos Oikonomopoulos]

(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]

(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]

This is a really useful patch, thanks @angelos.
I am retrying builds on ews to see if all is good.

[Caio Lima]

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 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]

Created attachment 403946 [details]
Patch

[Angelos Oikonomopoulos]

Created attachment 403952 [details]
Patch

[Angelos Oikonomopoulos]

Created attachment 403963 [details]
Patch

[Angelos Oikonomopoulos]

Created attachment 404091 [details]
Patch

[Yusuke Suzuki]

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]

(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]

Created attachment 416241 [details]
Patch

[Angelos Oikonomopoulos]

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 on attachment 416241 [details]
Patch

r=me, can you file a bug doing the same thing for `jsc-stress-test-writer-ruby.rb`?

[EWS]

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]

<rdar://problem/72831399>