Closed Bug 1475648 (arm64-baseline-tests) Opened Last year Closed 9 months ago

Fix jit-tests on Android ARM64 Baseline: Jit1-Jit10

Categories

(GeckoView :: General, defect, P1)

ARM64
Android
defect

Tracking

(geckoview62 wontfix, firefox-esr52 wontfix, firefox-esr60 wontfix, firefox61 wontfix, firefox62 wontfix, firefox63 wontfix, firefox64 wontfix, firefox65 wontfix, firefox66 affected)

RESOLVED FIXED
Tracking Status
geckoview62 --- wontfix
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- affected

People

(Reporter: cpeterson, Assigned: sstangl)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

(Keywords: leave-open, Whiteboard: [stockwell disabled][arm64:m1])

Attachments

(2 files, 1 obsolete file)

Bob is standing up testing on real Android devices. One of the test suites he have running is the jit test on Pixel 2 devices running Android 8.0 using 64bit builds as a Tier 3 job.

These are running as Tier 3 on mozilla-central, mozilla-inbound, autoland and try. Before we can begin to promote these tests to Tier 2 or Tier 1, we must get them to pass.

https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&filter-searchStr=jit&filter-tier=1&filter-tier=2&filter-tier=3

3 chunks perma fail: Jit5, Jit6 and Jit10
Meta bug 1446898 for ARM64 baseline JIT failures might cover some of these jit-test issues. Some example jit-test failures on these devices:

exception output: /data/local/tests/jit-tests/jit-tests/tests/debug/bug-1260728.js:10:3 Error: Assertion failed: got (void 0), expected 1

exception output: /data/local/tests/jit-tests/jit-tests/tests/debug/optimized-out-01.js:24:7 Error: Assertion failed: got "baseline", expected "ion"

TEST-UNEXPECTED-FAIL | tests/jit-test/jit-test/tests/parser/bug-1263355-33.js | /data/local/tests/jit-tests/jit-tests/tests/parser/bug-1263355-33.js line 21 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4 > eval line 4

exception output: /data/local/tests/jit-tests/jit-tests/lib/asserts.js:72:23 Error: Assertion failed: expected exception RuntimeError, got out of memory
ARM64 doesn't need to block Focus.
Priority: -- → P2
Whiteboard: [geckoview:p2] → [geckoview:fxr]
See Also: → 1439570
All of the jit tests are now orange on aarch64. See Bug 1496297 comment 37.

cpeterson: Can I just stop running these tests? They provide no value and consume resources and wear out the devices.
Flags: needinfo?(cpeterson)
See Also: → 1496297
(In reply to Bob Clary [:bc:] from comment #3)
> cpeterson: Can I just stop running these tests? They provide no value and
> consume resources and wear out the devices.

OK. I'll expand this bug to cover all the jit test failures on ARM64.
Flags: needinfo?(cpeterson)
Summary: Fix jit-tests on Android ARM64: Jit5, Jit6 and Jit10 → Fix jit-tests on Android ARM64: Jit1-Jit10
Whiteboard: [geckoview:fxr] → [geckoview:fxr:p2]
This is due to bug 1496297. That patch changed the command lines that we send to the JS shell processes that jit_test.py runs.

Maybe something in that patch interacts badly with --remote. Dan, can you see where we went wrong?
Flags: needinfo?(dminor)
Sean, sdetar says you are working on ARM64 in Q4. This bug tracks some jit test failures in ARM64 Baseline.
Assignee: nobody → sstangl
Flags: needinfo?(sstangl)
Depends on: 1439570
See Also: arm64-baseline, 1439570
I would like to turn them off on mozilla-central and just leave the ability to run them on try.

Ever since oct 10th:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&searchStr=android,jit&fromchange=b85ace8c5339f5f24e7d104b4a8146dc92bb694d&tochange=91b4c3687d7563244fbba0f58075779eb89259fb

and this merge to central:
https://hg.mozilla.org/mozilla-central/pushloghtml?changeset=bf31de5be0dcd71f3257485c66db5627ec3ed205

we have many more failures. Given this, I would really recommend turning these off on mozilla-central and leaving them on from users on try to run the tests until they are greened up and we can start running them in CI again.  As these tests are tier-3 (due to high number of perma failing tests for the last couple months) we missed when the tests became 100% orange.
https://treeherder.mozilla.org/#/jobs?repo=try&tier=1,2,3&group_state=expanded&revision=5f8c77565fa9ef9c267233289fc84a921dc6a26b

If you approve and r+, please go ahead and land this since I'll be out of the office in the morning so we can stop the orange bleeding.
Attachment #9017402 - Flags: review?(jmaher)
leave open after bug-1475648-disable-android-hw-jit.patch
Whiteboard: [geckoview:fxr:p2] → [geckoview:fxr:p2][leave-open]
Whiteboard: [geckoview:fxr:p2][leave-open][stockwell disable-recommended] → [geckoview:fxr:p2][leave-open][stockwell disabled]
(In reply to Jason Orendorff [:jorendorff] from comment #5)
> This is due to bug 1496297. That patch changed the command lines that we
> send to the JS shell processes that jit_test.py runs.
> 
> Maybe something in that patch interacts badly with --remote. Dan, can you
> see where we went wrong?

I'm sorry, it has been ages since I've looked at the test frameworks. I had a quick look and I don't think I can be much help here. Maybe :gbrown has some ideas?
Flags: needinfo?(dminor) → needinfo?(gbrown)
I see:

12:01:21     INFO -  exception output: -e:1:49 SyntaxError: missing ( before formal parameters:
12:01:21     INFO -  -e:1:49 if ((typeof dumpStringRepresentation !== function)) quit(59)
12:01:21     INFO -  -e:1:49 .................................................^
12:01:21     INFO -  -e:1:49 SyntaxError: missing ( before formal parameters:
12:01:21     INFO -  -e:1:49 if ((typeof dumpStringRepresentation !== function)) quit(59)
12:01:21     INFO -  -e:1:49 .................................................^-e:1:49 SyntaxError: missing ( before formal parameters:
12:01:21     INFO -  -e:1:49 if ((typeof dumpStringRepresentation !== function)) quit(59)
12:01:21     INFO -  -e:1:49 .................................................^Exit code: 3
12:01:21     INFO -  FAIL - basic/dumpStringRepresentation.js
12:01:21  WARNING -  TEST-UNEXPECTED-FAIL | tests/jit-test/jit-test/tests/basic/dumpStringRepresentation.js | -e:1:49 SyntaxError: missing ( before formal parameters: (code 3, args "") [0.2 s]

which I agree points to bug 1496297 and https://hg.mozilla.org/mozilla-central/rev/2a7a0b6b2bbf in particular.

I don't quite see the problem. Quoting? Will try a few things...
lets not check this in if :gbrown is looking into this today.
> I don't quite see the problem. Quoting? Will try a few things...

Yes -- typeof returns a string, so it should be `if ((typeof dumpStringRepresentation !== "function")) quit(59)`. So something is removing the quotes.
Flags: needinfo?(sstangl)
@Jason, that seems OK -- it's just wrapping the command in single-quotes.

The line in question actually uses single-quotes instead of double-quotes:

> js/src/jit-tests/tests/basic/dumpStringRepresentation.js:
> // |jit-test| skip-if: typeof dumpStringRepresentation !== 'function'

Semi-relatedly, the following looks wrong, given that the lhs should always be "string":

> tests/gc/bug1191756.js
> // |jit-test| skip-if: typeof 'oomAtAllocation' === 'undefined'
Oh, but it's not actually escaping internal characters. I see what you mean.
I got the jit-tests to run locally on a device and will look into the issue with escaping the command line.
Depends on: 1499511
Flags: needinfo?(gbrown)
We've landed a fix for the command line quoting in bug 1499511 so hopefully that issue is resolved.
Blocks: 1501584
The android hardware tests have been re-enabled after we appear to have resolved the failures due to downloads from tooltool and the taskcluster related breakage. The jit tests are currently only enabled on try for Pixel2 AArch64 since they have been perma orange and tier 3 for some time.

A try push <https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&author=bclary%40mozilla.com&group_state=expanded&fromchange=f13f8079d4c4af6e0a352be2dde454cf09d78dab&searchStr=android-hw%2Cjit&selectedJob=208100966> from today shows green jit{1..9} with only jit10 being orange.

You can run these tests on try via:

./mach try fuzzy --full --query "android-hw jittest"

When they green, we can re-enable them on mozilla-central at least.
(In reply to Bob Clary [:bc:] from comment #26)
> A try push
> <https://treeherder.mozilla.org/#/
> jobs?repo=try&tier=1%2C2%2C3&author=bclary%40mozilla.
> com&group_state=expanded&fromchange=f13f8079d4c4af6e0a352be2dde454cf09d78dab&
> searchStr=android-hw%2Cjit&selectedJob=208100966> from today shows green
> jit{1..9} with only jit10 being orange.

Clarification: the try push shows jit{1..9} have some green and some intermittent oranges, while jit10 is perma orange.
Blocks: 1098508
Test failures from "Android 8.0 Pixel2 AArch64 opt" with Baseline:

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=3ef89061cb3b7b0ca68ede7371abdc289410936a

Jit1
exception output: /data/local/tests/jit-tests/jit-tests/tests/TypedObject/fuzz4.js:6:3 Error: Assertion failed: got (void 0), expected ({0:0, 1:0, 2:0, 3:0, 4:0, 5:0, 6:0, 7:0, 8:0, 9:0})

Jit2
exception output: /data/local/tests/jit-tests/jit-tests/tests/auto-regress/bug710192.js:10:1 Error: Assertion failed: got (void 0), expected "1234"

Jit6
exception output: /data/local/tests/jit-tests/jit-tests/tests/debug/bug-1260728.js:10:3 Error: Assertion failed: got (void 0), expected 1

Jit10
exception output: /data/local/tests/jit-tests/jit-tests/lib/asserts.js:72:23 Error: Assertion failed: expected exception RuntimeError, got out of memory

The Jit3-5,9 failures look like they might be automation issues:

URLError: <urlopen error [Errno -3] Temporary failure in name resolution>
ERROR - The following files failed: 'host-utils-61.0a1.en-US.linux-x86_64.tar.gz'
Return code: 1
IOError: poll() gave POLLNVAL or POLLERR
Priority: P2 → P1
Whiteboard: [geckoview:fxr:p2][leave-open][stockwell disabled] → [geckoview:p2][stockwell disabled]
(In reply to Chris Peterson [:cpeterson] from comment #29)

> The Jit3-5,9 failures look like they might be automation issues:
> 
> URLError: <urlopen error [Errno -3] Temporary failure in name resolution>
> ERROR - The following files failed:
> 'host-utils-61.0a1.en-US.linux-x86_64.tar.gz'
> Return code: 1
> IOError: poll() gave POLLNVAL or POLLERR

This was from a try run from Friday. This automation issue has been worked around and should not be an issue going forward while the cached host-utils and minidump_stackwalk are still valid.

Use a test run from Monday for a more recent version:

<https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&group_state=expanded&revision=ead5af56741a679f99d596cb926ff583a88f7660&searchStr=jit>
Whiteboard: [geckoview:p2][stockwell disabled] → [stockwell disabled][arm64:m1]
Blocks: 1500611
Depends on: 1511618
Depends on: 1511615
Product: Firefox for Android → GeckoView
Once bug 1516913, bug 1511618 and bug 1511615 land, we will still have several failures which prevent us from running the jittests on android-hw in production. This patch disables the following tests

js/src/jit-test/tests/debug/DebuggeeWouldRun-02.js
js/src/jit-test/tests/parser/bug-1263355-6.js
js/src/jit-test/tests/pic/thisprop.js
js/src/jit-test/tests/wasm/atomic.js
js/src/jit-test/tests/wasm/baseline-abs-addr-opt.js
js/src/jit-test/tests/wasm/bce.js
js/src/jit-test/tests/wasm/memory.js
js/src/jit-test/tests/wasm/spec/float_memory.wast.js
js/src/jit-test/tests/wasm/spec/memory_redundancy.wast.js

on either arm7 or arm64 as appropriate. You can see the final result of all of these patches in the try run: <https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&author=bclary%40mozilla.com>

This will allow us to enable the jittests in production on android-hw.
Attachment #9033715 - Flags: review?(jmaher)
I'll fix the commit message to point to this bug.
I believe

js/src/jit-test/tests/wasm/atomic.js
js/src/jit-test/tests/wasm/baseline-abs-addr-opt.js
js/src/jit-test/tests/wasm/bce.js
js/src/jit-test/tests/wasm/memory.js
js/src/jit-test/tests/wasm/spec/float_memory.wast.js
js/src/jit-test/tests/wasm/spec/memory_redundancy.wast.js

should be covered by bug 1513231.

I'll file new bugs for

js/src/jit-test/tests/debug/DebuggeeWouldRun-02.js
js/src/jit-test/tests/parser/bug-1263355-6.js
js/src/jit-test/tests/pic/thisprop.js

and update the patch to point to the relevant bugs.
Comment on attachment 9033715 [details] [diff] [review]
bug-1454918-disable-jittests-bug684576.patch

Review of attachment 9033715 [details] [diff] [review]:
-----------------------------------------------------------------

I hope we leave this open or make sure the JS team knows what we are disabling.
Attachment #9033715 - Flags: review?(jmaher) → review+
Whiteboard: [stockwell disabled][arm64:m1] → [stockwell disabled][arm64:m1][leave-open]
After running another try run

<https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=fa8164c1159d182ffa6f978cb997a153a5af12c1>

I removed the DebuggeeWouldRun-02, bug-1263355-6, thisprop.js tests from the patch since they are not that reproducible and can be filed by the sheriffs if necessary.
Attachment #9033715 - Attachment is obsolete: true
Attachment #9033732 - Flags: review?(jmaher)
Pushed by bclary@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4688c503e2ee
disable jittests on android-hw-p2, r=jmaher.
Thanks, Bob. Running the passing jittests on real devices in automation is a great step forward.
Keywords: leave-open
Whiteboard: [stockwell disabled][arm64:m1][leave-open] → [stockwell disabled][arm64:m1]
This disables several probably-important wasm tests on ARM/ARM64.
Flags: needinfo?(luke)
Flags: needinfo?(lhansen)
Yes, I've asked Bob for further clarification over in bug 1513231.
Flags: needinfo?(lhansen)
This bug is about test failures with the ARM64 Baseline JIT. Test failures with the ARM64 Ion JIT are tracked in arm64-ion-tests bug 1187093.
Alias: arm64-baseline-tests
Summary: Fix jit-tests on Android ARM64: Jit1-Jit10 → Fix jit-tests on Android ARM64 Baseline: Jit1-Jit10
I think we can close this bug because the tests that Bob temporarily disabled are tracked in bug 1513231. AFAIU, ARM64 Baseline is passing all the other jit-tests.
Status: NEW → RESOLVED
Closed: 9 months ago
Flags: needinfo?(luke)
Resolution: --- → FIXED
Blocks: 1519351
No longer blocks: 1519351
You need to log in before you can comment on or make changes to this bug.