Closed Bug 1677045 Opened 7 months ago Closed 7 months ago

Remove JS_MORE_DETERMINISTIC in favor of a runtime option

Categories

(Core :: JavaScript Engine, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
85 Branch
Tracking Status
firefox84 --- wontfix
firefox85 --- fixed

People

(Reporter: decoder, Assigned: decoder)

References

Details

Attachments

(1 file)

For years, we have been using a special build with the JS_MORE_DETERMINISTIC define to do differential testing in our JS engine. In an effort to try and reduce build configurations, avoid build bustage and not having to add yet another build to fuzzfetch, I think we should replace this build mode with a runtime option, a build option isn't strictly necessary: Instead, in DEBUG mode we could check for a global runtime flag.

Instead, in DEBUG mode we could check for a global runtime flag.

I'm in favour of reducing the number of build configurations to test.

If this is debug-only, I guess we don't care about opt builds w/ determinism on? (we don't ship deterministic builds anyway)

(In reply to Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) from comment #2)

If this is debug-only, I guess we don't care about opt builds w/ determinism on? (we don't ship deterministic builds anyway)

I kept this debug-only because I don't want the runtime checks to influence performance in any way.

Priority: -- → P1
Severity: -- → N/A
Pushed by choller@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/750b066e600e
Replace JS_MORE_DETERMINISTIC with a runtime flag. r=jandem

Backed out for causing SM bustage in Iteration.cpp

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

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=HWak5zRVTXydpjEiqXdV9Q.0&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&revision=750b066e600ef3018f43b5019e99a8ad7fc56231

Failure log: https://treeherder.mozilla.org/logviewer?job_id=323299450&repo=autoland&lineNumber=5806

TinderboxPrint: rooting hazards<br/>1
[task 2020-12-02T12:52:16.922Z] TinderboxPrint: (unsafe references to unrooted GC pointers)<br/>85
[task 2020-12-02T12:52:16.922Z] TinderboxPrint: (unnecessary roots)<br/>1006
[task 2020-12-02T12:52:16.922Z] TinderboxPrint: missing expected hazards<br/>0
[task 2020-12-02T12:52:16.922Z] TinderboxPrint: heap write hazards<br/>0
[task 2020-12-02T12:52:16.924Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'b' of type 'jsid' live across GC call at js/src/vm/Iteration.cpp:428
[task 2020-12-02T12:52:16.924Z] TEST-UNEXPECTED-FAIL | hazards | 1 rooting hazards detected
[task 2020-12-02T12:52:16.924Z] TinderboxPrint: documentation<br/><a href='https://wiki.mozilla.org/Javascript:Hazard_Builds#Diagnosing_a_rooting_hazards_failure'>static rooting hazard analysis failures</a>, visit "Inspect Task" link for hazard details
[task 2020-12-02T12:52:16.924Z] + grab_artifacts
[task 2020-12-02T12:52:16.924Z] + local artifacts
[task 2020-12-02T12:52:16.924Z] + artifacts=/builds/worker/artifacts
[task 2020-12-02T12:52:16.924Z] + '[' -d /builds/worker/workspace/haz-js ']'
[task 2020-12-02T12:52:16.925Z] + cd /builds/worker/workspace/haz-js
[task 2020-12-02T12:52:16.925Z] + ls -lah
[task 2020-12-02T12:52:16.926Z] total 775M
[task 2020-12-02T12:52:16.926Z] drwxr-xr-x 3 worker worker 4.0K Dec 2 12:52

Flags: needinfo?(choller)

(In reply to Sandor Molnar from comment #5)

Backed out for causing SM bustage in Iteration.cpp

This is a pre-existing bug in the code, that we didn't detect because this code was not built in CI before. I'm testing a fix on try now.

Flags: needinfo?(choller)
Pushed by choller@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c51fbde2f226
Replace JS_MORE_DETERMINISTIC with a runtime flag. r=jandem
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → 85 Branch
You need to log in before you can comment on or make changes to this bug.