Closed Bug 1697904 Opened 4 years ago Closed 4 years ago

Migrate more JS prefs to StaticPrefs

Categories

(Core :: JavaScript Engine, task, P2)

Product:
Core
Component:
JavaScript Engine
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
88 Branch
Tracking Flags:
Tracking Status
firefox88 --- fixed

People

(Reporter: tcampbell, Assigned: tcampbell)

References

Details

(Keywords: perf-alert)

Attachments

(8 files, 1 obsolete file)

Bug 1697904 - Remove javascript.*.unsafe_eager_compilation prefs. r?iain!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1697904 - Make default values of unlisted javascript.options prefs explicit. r?jandem!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1697904 - Migrate JIT-enable prefs to StaticPrefs. r?jandem!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1697904 - Migrate JIT threshold prefs to StaticPrefs. r?jandem!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1697904 - Migrate more JIT prefs to StaticPrefs. r?jandem!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1697904 - Migrate the JIT spectre mitigation prefs to StaticPrefs. r?jandem!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1697904 - Cleanup StaticPrefList.yaml formatting of javascript prefs. r?jandem!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1697904 - Stop using RelaxedAtomicBool type for js prefs. r?jandem!
48 bytes, text/x-phabricator-request
Details | Review

This prevents typos in our code that reads prefs and opens up the option to use the YAML file to get away from our manual pref wrapping code.

These prefs are unlisted (eg not in about:config) and seem to hark back to a
different era. In practice, setting the appropriate JIT threshold to 0 does
the same thing and is much clearer.

These prefs are unlisted in all.js / StaticPrefs and so make sure that is
obvious from the code and that their default is explicit. These probably
should just become listed prefs in the future.

Depends on D108102

Use 'mirror: once' for these prefs to make it clear they are only read again
after a restart.

The "jit_trustedprincipals" pref is currently unlisted, so make that obvious
in the code and make the default value explicit.

Depends on D108103

Also relax the type on large_arraybuffers to plain bool since it is only read
once on startup.

Depends on D108105

To simplify this code, turns these prefs into unlisted prefs on MIPS
platforms since the JIT support is missing there. The JitOptions will
continue to default them to false on MIPS.

Depends on D108106

Attachment #9208542 - Attachment description: Bug 1697904 - Migrate the JIT spectre mitigation prefs to StaticPrefs. r?jandem!,iain! → Bug 1697904 - Migrate the JIT spectre mitigation prefs to StaticPrefs. r?jandem!

Depends on D108107

The top of the file has a clear formatting guide, so we should try to follow it.

Depends on D108125

We seem to have cargo-culted this in from nearby gfx code, but since we read
prefs only during the spidermonkey prefs callback this is a bit silly.

Depends on D108126

  • julian since he's trying to figure out a prefs problem we're having and the wasm patch almost certainly conflicts with whatever he's cooking together.

I'm happy to drop the WASM ones off the stack. They are from the non-startup pref set which is still very far switched over.

  • julian since he's trying to figure out a prefs problem we're having [..]
    That is bug 1697560.
Pushed by tcampbell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/17d4fbbd7a1e Remove javascript.*.unsafe_eager_compilation prefs. r=iain https://hg.mozilla.org/integration/autoland/rev/09df3a61027d Make default values of unlisted javascript.options prefs explicit. r=jandem https://hg.mozilla.org/integration/autoland/rev/2b0048582dcf Migrate JIT-enable prefs to StaticPrefs. r=jandem,KrisWright https://hg.mozilla.org/integration/autoland/rev/c8680ade8ec5 Migrate JIT threshold prefs to StaticPrefs. r=jandem https://hg.mozilla.org/integration/autoland/rev/d9a650b603f5 Migrate more JIT prefs to StaticPrefs. r=jandem https://hg.mozilla.org/integration/autoland/rev/87c31ed94b30 Migrate the JIT spectre mitigation prefs to StaticPrefs. r=jandem https://hg.mozilla.org/integration/autoland/rev/4aaab7a45b92 Cleanup StaticPrefList.yaml formatting of javascript prefs. r=jandem https://hg.mozilla.org/integration/autoland/rev/f37bc1afe77c Stop using RelaxedAtomicBool type for js prefs. r=jandem

Backed out 9 changesets (bug 1697935, bug 1697904) for assertion failure at StaticPrefList_layers.h and hardware acceleration related failures.

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=ddRad-TlTwKCWRs11yCtrQ.0&fromchange=3db292778c461ca8d7432cb00251dd25fe89183f&searchStr=linux%2C18.04%2Cx64%2Cdebug%2Cmochitests%2Ctest-linux1804-64%2Fdebug-mochitest-browser-chrome-e10s%2Cbc3&tochange=9a44b0f21c0ff2cded5a4c6cb1f30cbe166b1304
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&fromchange=3db292778c461ca8d7432cb00251dd25fe89183f&searchStr=windows%2C10%2Cx64%2Cwebrender%2Copt%2Cmochitests%2Ctest-windows10-64-qr%2Fopt-mochitest-browser-chrome-e10s%2Cbc1&selectedTaskRun=Sdope9pwT2WbzH8KRsD5WA.0&tochange=9a44b0f21c0ff2cded5a4c6cb1f30cbe166b1304

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

Failures logs:
https://treeherder.mozilla.org/logviewer?job_id=333090921&repo=autoland&lineNumber=13200

...
[task 2021-03-13T02:18:35.080Z] 02:18:35     INFO - TEST-OK | browser/components/aboutlogins/tests/browser/browser_vulnerableLoginAddedInSecondaryWindow.js | took 4619ms
[task 2021-03-13T02:18:35.101Z] 02:18:35     INFO - GECKO(3012) | [Child 3178: Main Thread]: I/DocShellAndDOMWindowLeak ++DOCSHELL 7fa4b70d2c00 == 2 [pid = 3178] [id = 12]
[task 2021-03-13T02:18:35.101Z] 02:18:35     INFO - GECKO(3012) | [Child 3178: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 7 (7fa4b7a43580) [pid = 3178] [serial = 27] [outer = 0]
[task 2021-03-13T02:18:35.101Z] 02:18:35     INFO - GECKO(3012) | [Child 3178: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 8 (7fa4b70d4400) [pid = 3178] [serial = 28] [outer = 7fa4b7a43580]
[task 2021-03-13T02:18:35.141Z] 02:18:35     INFO - checking window state
[task 2021-03-13T02:18:35.197Z] 02:18:35     INFO - GECKO(3012) | [Parent 3012: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 15 (7f80d6936000) [pid = 3012] [serial = 25] [outer = 7f80e17fb740]
[task 2021-03-13T02:18:35.204Z] 02:18:35     INFO - GECKO(3012) | [Child 3142, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111 (NS_ERROR_NOT_AVAILABLE): file /builds/worker/checkouts/gecko/caps/BasePrincipal.cpp:1349
[task 2021-03-13T02:18:35.220Z] 02:18:35     INFO - GECKO(3012) | [Parent 3012, Main Thread] WARNING: NS_ENSURE_TRUE(presShell) failed: file /builds/worker/checkouts/gecko/dom/base/nsGlobalWindowOuter.cpp:4253
[task 2021-03-13T02:18:35.228Z] 02:18:35     INFO - GECKO(3012) | [Parent 3012: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 16 (7f80d69ee400) [pid = 3012] [serial = 26] [outer = 7f80e17fb740]
...
...
...
[task 2021-03-13T02:18:46.169Z] 02:18:46     INFO - TEST-PASS | leakcheck | tab no leaks detected!
[task 2021-03-13T02:18:46.170Z] 02:18:46     INFO - runtests.py | Running tests: end.
[task 2021-03-13T02:18:46.190Z] 02:18:46     INFO - Buffered messages finished
[task 2021-03-13T02:18:46.191Z] 02:18:46     INFO - Running manifest: browser/components/enterprisepolicies/tests/browser/hardware_acceleration/browser.ini
[task 2021-03-13T02:18:46.191Z] 02:18:46     INFO - The following extra prefs will be set:
[task 2021-03-13T02:18:46.192Z] 02:18:46     INFO -   browser.policies.alternatePath='<test-root>/browser/components/enterprisepolicies/tests/browser/hardware_acceleration/disable_hardware_acceleration.json'
[task 2021-03-13T02:18:46.212Z] 02:18:46     INFO -  Setting pipeline to PAUSED ...
[task 2021-03-13T02:18:46.212Z] 02:18:46     INFO -  Pipeline is PREROLLING ...
[task 2021-03-13T02:18:46.213Z] 02:18:46     INFO -  Pipeline is PREROLLED ...
[task 2021-03-13T02:18:46.213Z] 02:18:46     INFO -  Setting pipeline to PLAYING ...
[task 2021-03-13T02:18:46.213Z] 02:18:46     INFO -  New clock: GstSystemClock
[task 2021-03-13T02:18:46.249Z] 02:18:46     INFO -  Got EOS from element "pipeline0".
[task 2021-03-13T02:18:46.249Z] 02:18:46     INFO -  Execution ended after 0:00:00.033400678
[task 2021-03-13T02:18:46.250Z] 02:18:46     INFO -  Setting pipeline to PAUSED ...
[task 2021-03-13T02:18:46.250Z] 02:18:46     INFO -  Setting pipeline to READY ...
[task 2021-03-13T02:18:46.250Z] 02:18:46     INFO -  (gst-launch-1.0:3253): GStreamer-CRITICAL **: 02:18:46.244: gst_object_unref: assertion '((GObject *) object)->ref_count > 0' failed
[task 2021-03-13T02:18:46.250Z] 02:18:46     INFO -  Setting pipeline to NULL ...
[task 2021-03-13T02:18:46.250Z] 02:18:46     INFO -  Freeing pipeline ...
[task 2021-03-13T02:18:46.566Z] 02:18:46     INFO - PID 3266 | pk12util: PKCS12 IMPORT SUCCESSFUL
[task 2021-03-13T02:18:46.667Z] 02:18:46     INFO - Increasing default timeout to 90 seconds
[task 2021-03-13T02:18:46.682Z] 02:18:46     INFO - MochitestServer : launching [u'/builds/worker/workspace/build/tests/bin/xpcshell', '-g', '/builds/worker/workspace/build/application/firefox', '-f', '/builds/worker/workspace/build/tests/bin/components/httpd.js', '-e', "const _PROFILE_PATH = '/tmp/tmpR_vpcc.mozrunner'; const _SERVER_PORT = '8888'; const _SERVER_ADDR = '127.0.0.1'; const _TEST_PREFIX = undefined; const _DISPLAY_RESULTS = false;", '-f', '/builds/worker/workspace/build/tests/mochitest/server.js']
[task 2021-03-13T02:18:46.683Z] 02:18:46     INFO - runtests.py | Server pid: 3272
[task 2021-03-13T02:18:46.692Z] 02:18:46     INFO - runtests.py | Websocket server pid: 3275
[task 2021-03-13T02:18:46.715Z] 02:18:46     INFO - runtests.py | SSL tunnel pid: 3279
[task 2021-03-13T02:18:46.796Z] 02:18:46     INFO -  Couldn't convert chrome URL: chrome://branding/locale/brand.properties
[task 2021-03-13T02:18:46.843Z] 02:18:46     INFO -  [Parent 3272, Main Thread] WARNING: Could not get the program name for a cubeb stream.: 'NS_SUCCEEDED(rv)', file /builds/worker/checkouts/gecko/dom/media/CubebUtils.cpp:377
[task 2021-03-13T02:18:46.869Z] 02:18:46     INFO - runtests.py | Running with scheme: http
[task 2021-03-13T02:18:46.869Z] 02:18:46     INFO - runtests.py | Running with e10s: True
[task 2021-03-13T02:18:46.870Z] 02:18:46     INFO - runtests.py | Running with fission: False
[task 2021-03-13T02:18:46.870Z] 02:18:46     INFO - runtests.py | Running with cross-origin iframes: False
[task 2021-03-13T02:18:46.870Z] 02:18:46     INFO - runtests.py | Running with serviceworker_e10s: True
[task 2021-03-13T02:18:46.870Z] 02:18:46     INFO - runtests.py | Running with socketprocess_e10s: False
[task 2021-03-13T02:18:46.871Z] 02:18:46     INFO - runtests.py | Running tests: start.
[task 2021-03-13T02:18:46.871Z] 02:18:46     INFO - 
[task 2021-03-13T02:18:46.890Z] 02:18:46     INFO - Application command: /builds/worker/workspace/build/application/firefox/firefox -marionette -foreground -profile /tmp/tmpR_vpcc.mozrunner
[task 2021-03-13T02:18:46.905Z] 02:18:46     INFO - runtests.py | Application pid: 3294
[task 2021-03-13T02:18:46.905Z] 02:18:46     INFO - TEST-INFO | started process GECKO(3294)
[task 2021-03-13T02:18:46.929Z] 02:18:46     INFO - GECKO(3294) | ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to /tmp/tmpR_vpcc.mozrunner/runtests_leaks.log
[task 2021-03-13T02:18:46.929Z] 02:18:46     INFO - GECKO(3294) | [3294, Main Thread] WARNING: XPCOM_MEM_BLOAT_LOG is set, disabling native allocations.: file /builds/worker/checkouts/gecko/tools/profiler/core/platform.cpp:251
[task 2021-03-13T02:18:47.228Z] 02:18:47     INFO - GECKO(3294) | Assertion failure: staticPrefValue == preferenceValue (Preference 'layers.acceleration.disabled' got modified since StaticPrefs::layers_acceleration_disabled_AtStartup_DoNotUseDirectly was initialized. Consider using an `always` mirror kind instead), at /builds/worker/workspace/obj-build/dist/include/mozilla/StaticPrefList_layers.h:8
[task 2021-03-13T02:18:47.229Z] 02:18:47     INFO - GECKO(3294) | #01: NotifyCallbacks(nsTString<char> const&, PrefWrapper const*) [modules/libpref/Preferences.cpp:1721]
[task 2021-03-13T02:18:47.230Z] 02:18:47     INFO - GECKO(3294) | #02: pref_SetPref(nsTString<char> const&, mozilla::PrefType, mozilla::PrefValueKind, PrefValue, bool, bool, bool) [modules/libpref/Preferences.cpp:1642]
[task 2021-03-13T02:18:47.231Z] 02:18:47     INFO - GECKO(3294) | #03: mozilla::Preferences::SetBool(char const*, bool, mozilla::PrefValueKind) [modules/libpref/Preferences.cpp:4883]
...

https://treeherder.mozilla.org/logviewer?job_id=333091866&repo=autoland&lineNumber=3770

[task 2021-03-13T02:22:30.786Z] 02:22:30     INFO - TEST-START | browser/components/enterprisepolicies/tests/browser/hardware_acceleration/browser_policy_hardware_acceleration.js
[task 2021-03-13T02:22:30.791Z] 02:22:30     INFO - TEST-INFO | started process screenshot
[task 2021-03-13T02:22:30.874Z] 02:22:30     INFO - TEST-INFO | screenshot: exit 0
[task 2021-03-13T02:22:30.875Z] 02:22:30     INFO - Buffered messages logged at 02:22:30
[task 2021-03-13T02:22:30.875Z] 02:22:30     INFO - Entering test bound test_policy_hardware_acceleration
[task 2021-03-13T02:22:30.875Z] 02:22:30     INFO - Buffered messages finished
[task 2021-03-13T02:22:30.875Z] 02:22:30     INFO - TEST-UNEXPECTED-FAIL | browser/components/enterprisepolicies/tests/browser/hardware_acceleration/browser_policy_hardware_acceleration.js | Hardware acceleration disabled - 
[task 2021-03-13T02:22:30.875Z] 02:22:30     INFO - Stack trace:
[task 2021-03-13T02:22:30.875Z] 02:22:30     INFO - chrome://mochikit/content/browser-test.js:test_ok:1331
[task 2021-03-13T02:22:30.875Z] 02:22:30     INFO - chrome://mochitests/content/browser/browser/components/enterprisepolicies/tests/browser/hardware_acceleration/browser_policy_hardware_acceleration.js:test_policy_hardware_acceleration:9
[task 2021-03-13T02:22:30.876Z] 02:22:30     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1089
[task 2021-03-13T02:22:30.876Z] 02:22:30     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1129
[task 2021-03-13T02:22:30.876Z] 02:22:30     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:949
[task 2021-03-13T02:22:30.876Z] 02:22:30     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:1037
[task 2021-03-13T02:22:30.877Z] 02:22:30     INFO - Leaving test bound test_policy_hardware_acceleration
[task 2021-03-13T02:22:30.877Z] 02:22:30     INFO - GECKO(7608) | MEMORY STAT | vsize 2104141MB | vsizeMaxContiguous 65691955MB | residentFast 237MB | heapAllocated 90MB
[task 2021-03-13T02:22:30.877Z] 02:22:30     INFO - TEST-OK | browser/components/enterprisepolicies/tests/browser/hardware_acceleration/browser_policy_hardware_acceleration.js | took 69ms
[task 2021-03-13T02:22:30.877Z] 02:22:30     INFO - checking window state
[task 2021-03-13T02:22:30.887Z] 02:22:30     INFO - GECKO(7608) | Completed ShutdownLeaks collections in process 10708
[task 2021-03-13T02:22:30.887Z] 02:22:30     INFO - TEST-START | Shutdown
Flags: needinfo?(tcampbell)

The test failures are for the prefs that follow JS prefs, so likely there is a bug in libpref that I need to track down. Landing the first few patches that don't touch the pref file to avoid them rotting more.

Keywords: leave-open
Pushed by tcampbell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b5b22dbbf9e1 Remove javascript.*.unsafe_eager_compilation prefs. r=iain https://hg.mozilla.org/integration/autoland/rev/79dd36ae4a18 Make default values of unlisted javascript.options prefs explicit. r=jandem

Looks like the use of mirror: once caused the StaticPrefs::InitOncePref() call to happen earlier than it does today. This breaks EnterprisePolicies which use JS to override certain prefs pretty early in startup, but not before JS initializes.

For these JS startup prefs, we could switch them to always and add a comment in the YAML that they are startup prefs.

Flags: needinfo?(tcampbell)

For this bug I'll replace mirror: once with mirror: always; do_not_use_directly: true and add comment explaining that LoadStartupJSPrefs is responsible for a separate mirroring system within spidermonkey.

Pushed by tcampbell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fe3aebebb290 Migrate JIT-enable prefs to StaticPrefs. r=jandem,KrisWright https://hg.mozilla.org/integration/autoland/rev/5a212b5e1cea Migrate JIT threshold prefs to StaticPrefs. r=jandem https://hg.mozilla.org/integration/autoland/rev/f97b1a06801e Migrate more JIT prefs to StaticPrefs. r=jandem https://hg.mozilla.org/integration/autoland/rev/39e442ab0179 Migrate the JIT spectre mitigation prefs to StaticPrefs. r=jandem https://hg.mozilla.org/integration/autoland/rev/b32ee5c655c6 Cleanup StaticPrefList.yaml formatting of javascript prefs. r=jandem https://hg.mozilla.org/integration/autoland/rev/7281f9859db9 Stop using RelaxedAtomicBool type for js prefs. r=jandem
Keywords: leave-open

This set should cover the startup prefs. Will create a new bug for the next round later since merge is coming up.

== Change summary for alert #29289 (as of Tue, 16 Mar 2021 14:42:01 GMT) ==

Improvements:

Ratio Suite Test Platform Options Absolute values (old vs new)
4% raptor-ares6-firefox linux1804-64-shippable nocondprof 62.30 -> 59.77
4% raptor-ares6-firefox linux1804-64-shippable nocondprof 62.10 -> 59.80
3% raptor-ares6-firefox macosx1015-64-shippable-qr nocondprof webrender 56.45 -> 54.68
3% raptor-ares6-firefox windows10-64-shippable-qr nocondprof webrender 54.96 -> 53.27
2% raptor-sunspider-firefox macosx1015-64-shippable-qr nocondprof webrender 173.52 -> 169.43

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=29289

Keywords: perf-alert

NOTE: The perf improvement is due to an issue in Bug 1697935 breaking the spectre object mitigations. They are fixed again now that the complete stack here has landed and we'll see the perf numbers revert soon. Hopefully we once Fission is shipping 100% we can let off these mitigations and get a bit of perf.

== Change summary for alert #29284 (as of Mon, 15 Mar 2021 20:05:14 GMT) ==

Improvements:

Ratio Suite Test Platform Options Absolute values (old vs new)
8% kraken linux1804-64-shippable-qr e10s stylo webrender 1,421.17 -> 1,304.31
8% kraken linux1804-64-shippable e10s stylo 1,415.18 -> 1,305.71
6% perf_reftest_singletons getElementById-1.html windows10-64-shippable-qr e10s stylo webrender 48.88 -> 46.16
5% perf_reftest_singletons getElementById-1.html windows10-64-shippable e10s stylo 48.83 -> 46.19
4% kraken macosx1015-64-shippable-qr e10s stylo webrender 1,513.49 -> 1,450.43

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=29284

Attachment #9208564 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: