1677045 - Remove JS_MORE_DETERMINISTIC in favor of a runtime option

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P1)

Description

[Christian Holler (:decoder)]

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.

Comment 1

[Christian Holler (:decoder)]

Attachment: Bug 1677045 - Replace JS_MORE_DETERMINISTIC with a runtime flag. r?jandem

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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)

Comment 3

[Christian Holler (:decoder)]

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

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/750b066e600e
Replace JS_MORE_DETERMINISTIC with a runtime flag. r=jandem

Comment 5

[Sandor Molnar[:smolnar]]

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

Comment 6

[Christian Holler (:decoder)]

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c51fbde2f226
Replace JS_MORE_DETERMINISTIC with a runtime flag. r=jandem

Comment 8

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/c51fbde2f226