Bug 200789 – mozilla-tests.yaml/js1_5/Array/regress-101964.js is frequently failing on JSC EWS bots

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Ryan Haddad]

mozilla-tests.yaml/js1_5/Array/regress-101964.js is a flaky failure on JSC EWS bots.

Here are examples:
https://bugs.webkit.org/buglist.cgi?bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&bug_status=RESOLVED&bug_status=VERIFIED&bug_status=CLOSED&f0=OP&f1=OP&f2=product&f3=component&f4=alias&f5=short_desc&f7=content&f8=CP&f9=CP&j1=OR&list_id=4991060&o2=substring&o3=substring&o4=substring&o5=substring&o6=substring&o7=matches&query_format=advanced&v2=mozilla-tests.yaml%2Fjs1_5%2FArray%2Fregress-101964.js&v3=mozilla-tests.yaml%2Fjs1_5%2FArray%2Fregress-101964.js&v4=mozilla-tests.yaml%2Fjs1_5%2FArray%2Fregress-101964.js&v5=mozilla-tests.yaml%2Fjs1_5%2FArray%2Fregress-101964.js&v6=mozilla-tests.yaml%2Fjs1_5%2FArray%2Fregress-101964.js&v7=%22mozilla-tests.yaml%2Fjs1_5%2FArray%2Fregress-101964.js%22

[Radar WebKit Bug Importer]

<rdar://problem/54361916>

[Alexey Proskuryakov]

There were 15 false positives because of this on EWS in a month, so worth looking into soon.

[Alexey Proskuryakov]

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

[Alexey Proskuryakov]

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

[Keith Miller]

Let's just delete that test... I'm assuming it's failing because it's being run with dozens of other processes so it happens to get scheduled weirdly.

[Alexey Proskuryakov]

Indeed! Testing that something takes under 100 ms makes no sense when running many tests in parallel.

mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-baseline: STATUS: Performance: truncating even very large arrays should be fast!
mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-baseline: FAILED!: [reported from test()] Section 1 of test -
mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-baseline: FAILED!: [reported from test()] Expected value 'Truncation took less than 100 ms', Actual value 'Truncation took 193 ms'
mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-baseline: FAILED!: [reported from test()]

[Aakash Jain]

This test flaked again in https://ews-build.webkit.org/#/builders/26/builds/469


mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-dfg-eager-no-cjit-validate-phases: Detected failures:
mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-dfg-eager-no-cjit-validate-phases: BUGNUMBER: 101964
mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-dfg-eager-no-cjit-validate-phases: STATUS: Performance: truncating even very large arrays should be fast!
mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-dfg-eager-no-cjit-validate-phases: FAILED!: [reported from test()] Section 1 of test -
mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-dfg-eager-no-cjit-validate-phases: FAILED!: [reported from test()] Expected value 'Truncation took less than 100 ms', Actual value 'Truncation took 336 ms'


Note that these tests are running on Raspberry Pi on Igalia JSC queues (ARMv7 and MIPSEL).

[Aakash Jain]

Flaked again in: https://ews-build.webkit.org/#/builders/26/builds/527

[Paulo Matos]

I have noticed, as Aakash that this test keeps passing/failing on mips and armv7. I have opened bug 204746 to skip it on these architectures, but I am happy if it's removed altogether.

[Guillaume Emont]

As Paulo just said, it flakes quite regularly on mips (though our mips test devices are not raspberry pis, obviously, but they're quite slow ci20 dev boards) and  on armv7 (rpi3s). Running the test on its own with nothing else running on the board it always passes though, failures only seem to happen while running other tests.

Maybe raising the timeout could be a solution? I think on mips when it fails, it's still always under 200ms, I guess we could do 300ms to be on the safe side, and from what I understand, if the regression it's testing would happen, the truncation would take an order of magnitude more than that, so a timeout of 300-500ms might be more appropriate for this test?

[Aakash Jain]

(In reply to Guillaume Emont from comment #10)
> Maybe raising the timeout could be a solution? I think on mips when it
> fails, it's still always under 200ms, I guess we could do 300ms to be on the safe side
On armv7 it took 336ms in https://ews-build.webkit.org/#/builders/26/builds/469/steps/9/logs/stdio (line 37271). So, for armv7 we would either need to skip/disable this test or choose higher timeout.

[Keith Miller]

I think we should just get rid of this test. We don't run tests in isolation so even on high power machines the system could and likely does suspend this test to run other tests.

[Keith Miller]

Created attachment 384634 [details]
Patch

[Aakash Jain]

Should we delete https://trac.webkit.org/browser/webkit/trunk/JSTests/mozilla/js1_5/Array/regress-101964.js as well?

[Mark Lam]

(In reply to Aakash Jain from comment #14)
> Should we delete
> https://trac.webkit.org/browser/webkit/trunk/JSTests/mozilla/js1_5/Array/
> regress-101964.js as well?

I was just looking at the test.  It seems strange that it should ever time out.  I suggest filing a bug to investigate why it times out.

[Mark Lam]

(In reply to Mark Lam from comment #15)
> (In reply to Aakash Jain from comment #14)
> > Should we delete
> > https://trac.webkit.org/browser/webkit/trunk/JSTests/mozilla/js1_5/Array/
> > regress-101964.js as well?
> 
> I was just looking at the test.  It seems strange that it should ever time
> out.  I suggest filing a bug to investigate why it times out.

Sorry.  I mean I don't see why the test should ever fail.  It's testing a trivial operation.  I think we should do some due dilligence to make sure there's not some latent perf bug here.

[Keith Miller]

(In reply to Mark Lam from comment #16)
> (In reply to Mark Lam from comment #15)
> > (In reply to Aakash Jain from comment #14)
> > > Should we delete
> > > https://trac.webkit.org/browser/webkit/trunk/JSTests/mozilla/js1_5/Array/
> > > regress-101964.js as well?
> > 
> > I was just looking at the test.  It seems strange that it should ever time
> > out.  I suggest filing a bug to investigate why it times out.
> 
> Sorry.  I mean I don't see why the test should ever fail.  It's testing a
> trivial operation.  I think we should do some due dilligence to make sure
> there's not some latent perf bug here.

It could fail because the os schedules so other tests to run between the time checks.

[Ross Kirsling]

Interestingly, we already raised the threshold from 50ms to 100ms three years ago:
https://github.com/WebKit/webkit/commit/50c9e3155e5fa74b2579f8240fd00afbda4a00ae

[Mark Lam]

(In reply to Keith Miller from comment #17)
> (In reply to Mark Lam from comment #16)
> > (In reply to Mark Lam from comment #15)
> > > (In reply to Aakash Jain from comment #14)
> > > > Should we delete
> > > > https://trac.webkit.org/browser/webkit/trunk/JSTests/mozilla/js1_5/Array/
> > > > regress-101964.js as well?
> > > 
> > > I was just looking at the test.  It seems strange that it should ever time
> > > out.  I suggest filing a bug to investigate why it times out.
> > 
> > Sorry.  I mean I don't see why the test should ever fail.  It's testing a
> > trivial operation.  I think we should do some due dilligence to make sure
> > there's not some latent perf bug here.
> 
> It could fail because the os schedules so other tests to run between the
> time checks.

If this is the case, then we can trivially solve this by changing the test to use CPU time instead of wall clock time.  That should also prove that there's nothing else wrong.  I'll prepare a patch.

[Mark Lam]

Created attachment 384646 [details]
proposed patch.

[Mark Lam]

Created attachment 384647 [details]
proposed patch.

[Keith Miller]

Comment on attachment 384647 [details]
proposed patch.

r=me.

[Mark Lam]

Comment on attachment 384647 [details]
proposed patch.

Thanks for the review.  Landing now.

[WebKit Commit Bot]

Comment on attachment 384647 [details]
proposed patch.

Clearing flags on attachment: 384647

Committed r253008: <https://trac.webkit.org/changeset/253008>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Guillaume Emont]

(In reply to Mark Lam from comment #19)
> (In reply to Keith Miller from comment #17)
> > 
> > It could fail because the os schedules so other tests to run between the
> > time checks.
> 
> If this is the case, then we can trivially solve this by changing the test
> to use CPU time instead of wall clock time.  That should also prove that
> there's nothing else wrong.  I'll prepare a patch.

Looking at the mips bot results, this seems to have solved it: the test lately was failing every second run, and it's now been 16 runs without it failing.
Good idea, thanks!