Closed Bug 1507721 Opened 2 years ago Closed 2 years ago

Try to simplify AutoEnterOOMUnsafeRegion

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: jandem, Assigned: jandem)

Details

Attachments

(1 file)

AutoEnterOOMUnsafeRegion ctor/dtor modifying the simulation's max-checks field is a bit hard to reason about and I'm not sure it's always doing the right thing. It might be simpler to add a bool flag and use that instead.
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/1d50d21da50d
Simplify AutoEnterOOMUnsafeRegion by adding an explicit inUnsafeRegion_ flag to the OOM simulator. r=jonco
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&group_state=expanded&searchStr=spidermonkey&revision=1d50d21da50d91de52719decd7bf3e942a9ac89f&selectedJob=212257022

Failure log: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&group_state=expanded&searchStr=spidermonkey&revision=1d50d21da50d91de52719decd7bf3e942a9ac89f&selectedJob=212257022

Backout link: https://hg.mozilla.org/integration/autoland/rev/dad98317fee0ff692f30d6f0c845a9241f0d5de4

TEST-PASS | js\src\jit-test\tests\modules\dynamic-import-module.js | Success (code 0, args "--baseline-eager") [0.3 s]
{"action": "test_start", "jitflags": "--baseline-eager", "pid": 3724, "source": "jittests", "test": "modules\\dynamic-import-module.js", "thread": "main", "time": 1542397512.124}
{"action": "test_end", "extra": {"jitflags": "--baseline-eager"}, "jitflags": "--baseline-eager", "message": "Success", "pid": 3724, "source": "jittests", "status": "PASS", "test": "modules\\dynamic-import-module.js", "thread": "main", "time": 1542397512.387}
Exit code: 1
TIMEOUT - modules\dynamic-import-oom.js
TEST-UNEXPECTED-FAIL | js\src\jit-test\tests\modules\dynamic-import-oom.js | Timeout (code 1, args "") [150.0 s]
{"action": "test_start", "jitflags": "", "pid": 3724, "source": "jittests", "test": "modules\\dynamic-import-oom.js", "thread": "main", "time": 1542397512.373}
{"action": "test_end", "extra": {"jitflags": ""}, "jitflags": "", "message": "Timeout", "pid": 3724, "source": "jittests", "status": "FAIL", "test": "modules\\dynamic-import-oom.js", "thread": "main", "time": 1542397662.386}
INFO exit-status     : 1
INFO timed-out       : True
TEST-PASS | js\src\jit-test\tests\modules\dynamic-import-oom.js | Success (code 0, args "--ion-eager --ion-offthread-compile=off") [1.0 s]
{"action": "test_start", "jitflags": "--ion-eager --ion-offthread-compile=off", "pid": 3724, "source": "jittests", "test": "modules\\dynamic-import-oom.js", "thread": "main", "time": 1542397662.407}
{"action": "test_end", "extra": {"jitflags": "--ion-eager --ion-offthread-compile=off"}, "jitflags": "--ion-eager --ion-offthread-compile=off", "message": "Success", "pid": 3724, "source": "jittests", "status": "PASS", "test": "modules\\dynamic-import-oom.js", "thread": "main", "time": 1542397663.453}
Exit code: 1
TIMEOUT - modules\dynamic-import-oom.js
Flags: needinfo?(jdemooij)
dynamic-import-oom.js becomes slower now when oomTest is testing the THREAD_TYPE_ION thread type. We do a lot more iterations, I think this is because the old code bumped maxChecks too much in the AutoEnterOOMUnsafeRegion destructor.

I'll look into it more but I may add `|jit-test| --ion-offthread-compile=off` to dynamic-import-oom.js just to unblock this - it does fix correctness issues in our OOM testing infrastructure I think.
I added some logging and yes, before this patch we would for instance set maxChecks to 9512167 in FailureSimulator::simulateFailureAfter but then at the end of the iteration it would be 9512548 so we incorrectly added 381 to it and this would then confuse stopSimulating => HadSimulatedOOM into thinking we're done. With the patch this no longer happens so the test runs slower, hence the timeout.
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7987651658f0
Simplify AutoEnterOOMUnsafeRegion by adding an explicit inUnsafeRegion_ flag to the OOM simulator. r=jonco
Flags: needinfo?(jdemooij)
https://hg.mozilla.org/mozilla-central/rev/7987651658f0
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.