Closed Bug 1698102 Opened 4 years ago Closed 6 months ago

Revisit SpiderMonkey pref management

Categories

(Core :: JavaScript Engine, task, P2)

task

Tracking

()

RESOLVED DUPLICATE of bug 1877193

People

(Reporter: tcampbell, Unassigned)

References

(Blocks 1 open bug)

Details

Currently SpiderMonkey does not directly use entry points to libpref and instead we use mechanisms such as LoadStartupJSPrefs to convert libpref preferences into SM-specific enums and internal state. This has a lot of boilerplate and is easy to lose sync in edge cases around other threads and runtimes.

  1. We should try to migrate our javascript.options. prefs from all.js to the StaticPrefs system. This provides named accessor methods in C++ which avoids silly bugs with preferences names. This also gives us the ability to use YAML parser or macro tricks to remove a lot of the spidermonkey boilerplate. In Bug 1697904, I've started migrating some of our prefs.

  2. Investigate if we can reasonably run the libpref YAML parser as part of shell / pkg builds. This would give us access to https://searchfox.org/mozilla-central/source/__GENERATED__/__linux64__/modules/libpref/init/StaticPrefList_javascript.h which we could use for our usual macro tricks. If not, we could write a simple Python script to parse the YAML ourselves (similar to CacheIR YAML file).

  3. To keep Spidermonkey <-> libpref coupling minimal (for now), we can use this header file to generate a js::StaticPrefs shadow structure. The existing pref sync code in XPCJSContext.cpp would continue to be used to mirror from libpref to the shadow-prefs, but we could use macro tricks to remove boilerplate.

  4. The JSAPI pref enums would be removed. Embedders would probably need to directly interact with the shadow prefs in some way. Alternatively, we could just rename those enum values and auto-generate using the same pref macros.

  5. Within SpiderMonkey, we'd use the libpref conventions that Gecko does and directly reference the js::StaticPrefs::javascript_options_baselinejit_AtStartup() prefs where we need them. This would allow consistent behaviour across threads and workers. Some care might need to be taken for workers to sort out atomics issues, but that seems solvable.

It might be possible to shorten the names using namespace tricks. As a first cut, it might be nice to keep it explicit and consistent with Gecko though.

Additionally, if we go this route, we might want to clean up jsshell options to map to more of a --setpref model. For that we could have an optional implicit javascript.options. prefix so it doesn't have to be typed out.

A shell flag like --baseline-eager would basically be an alias to --setpref javascript.options.baseline.threshold=0. We could describe this in the C++ code of shell as an alias, and add it to the help string.

This is... mostly done I think? Or are there some subtasks here we should still split out?

Flags: needinfo?(jdemooij)
Blocks: sm-api
No longer blocks: sm-meta

(In reply to Matthew Gaudet (he/him) [:mgaudet] from comment #2)

This is... mostly done I think? Or are there some subtasks here we should still split out?

Yes I think this is done now. What was implemented in bug 1877193 is actually very similar to what Ted proposed in this bug, including the --setpref idea in comment 1. We just strip the javascript.options. prefix in most places.

What's left is converting older prefs to the new system, but I think we can do that in other bugs.

Status: NEW → RESOLVED
Closed: 6 months ago
Duplicate of bug: 1877193
Flags: needinfo?(jdemooij)
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.