Per measurements in bug 941818, this would save a lot of space in smaller worker runtimes, and, in combination with sharing atoms state between runtimes, would accomplish much the same memory usage reductions as using a single runtime for both the main thread and workers (bug 941818, a much larger task). Since self hosted state is generally read only this shouldn't be terribly difficult, and the main difficulties will be in handling the places where self hosted state is not read only but is lazily instantiated. Bug 941818 has some details about this.

I discussed this with Waldo and Luke, and it doesn't look like there are any fundamental problems. We currently have the ability to execute scripts in the self-hosting compartment, but we can get rid of that if needed. In that case, we could explicitly tenure and make the entire self-hosting compartment read-only and create it once per process. all runtimes would get a reference to it so they can access it for lazily cloning self-hosted functions. I don't know how accessing the values we clone over would work, though. The accessing thread shouldn't acquire the compilation lock, as we guarantee that the accessed data doesn't change. There are various places that assert js::CurrentThreadCanReadCompilationData, though, so we'd have to work around that.

This patch applies on top of the one in bug 964059, and is pretty straightforward. On top of bug 964059 this reduces an empty worker's memory usage an additional .41 MB, to .23 MB: 0.23 MB (00.30%) -- worker(script.js, 0x7f88dea53800) ├──0.12 MB (00.15%) -- zone(0x7f88dea7a800) │ ├──0.07 MB (00.09%) ++ compartment(web-worker) │ ├──0.03 MB (00.04%) ── unused-gc-things │ ├──0.01 MB (00.01%) ── type-pool │ └──0.00 MB (00.01%) ── sundries/gc-heap ├──0.07 MB (00.09%) -- runtime │ ├──0.03 MB (00.04%) -- gc │ │ ├──0.03 MB (00.04%) ── marker │ │ ├──0.00 MB (00.00%) ── nursery │ │ └──0.00 MB (00.00%) ++ store-buffer │ ├──0.03 MB (00.04%) ── runtime-object │ ├──0.00 MB (00.01%) ── atoms-table │ ├──0.00 MB (00.01%) ── dtoa │ ├──0.00 MB (00.00%) ── contexts │ ├──0.00 MB (00.00%) ── script-data │ ├──0.00 MB (00.00%) ── script-sources │ ├──0.00 MB (00.00%) ++ code │ ├──0.00 MB (00.00%) ── interpreter-stack │ ├──0.00 MB (00.00%) ── math-cache │ ├──0.00 MB (00.00%) ── regexp-data │ ├──0.00 MB (00.00%) ── source-data-cache │ └──0.00 MB (00.00%) ── temporary ├──0.03 MB (00.04%) -- gc-heap │ ├──0.03 MB (00.04%) ── chunk-admin │ ├──0.00 MB (00.00%) ── unused-arenas │ └──0.00 MB (00.00%) ── unused-chunks └──0.01 MB (00.02%) -- zone(0x7f88dea76000) ├──0.01 MB (00.02%) ++ sundries └──0.00 MB (00.00%) ++ compartment(web-worker-atoms) Except for the .03 MB of GC marker stuff (would be nice to be able to decommit that memory), this is pretty much in line with the estimate in bug 941818 comment 6.

Comment on attachment 8375171 [details] [diff] [review] patch Review of attachment 8375171 [details] [diff] [review]: ----------------------------------------------------------------- Nice! saving .41MB per worker is quite amazing, really. r=me with feedback addressed ::: js/src/frontend/BytecodeCompiler.cpp @@ +165,5 @@ > } > > +ScriptSourceObject * > +frontend::CreateScriptSourceObject(ExclusiveContext *cx, > + const ReadOnlyCompileOptions &options) This fits into one line, now ::: js/src/jscompartment.h @@ +262,5 @@ > void sweepCallsiteClones(); > > + /* > + * Lazily initialized script source object to use for scripts cloned > + * from the self hosting global. Uber-nit: self-hosting ::: js/src/jsscript.cpp @@ +2614,5 @@ > } > } > > + /* > + * Wrap the script source object as needed. Self hosted scripts may be Uber-nit: self-hosted ::: js/src/vm/Runtime.h @@ +910,5 @@ > WTF::BumpPointerAllocator *bumpAlloc_; > js::jit::JitRuntime *jitRuntime_; > > + /* > + * Self hosting state cloned on demand into other compartments. Shared with the parent Uber-nit: Self-hosting ::: js/src/vm/SelfHosting.cpp @@ +780,5 @@ > + * As that object is inaccessible to client code, the lookups are > + * guaranteed to return the original objects, ensuring safe implementation > + * of self-hosted builtins. > + * > + * Additionally, the special syntax _CallFunction(receiver, ...args, fun) For better or worse, it's "callFunction(fun, receiver, ...args)". Pre-existing, I know, but would be good to fix. @@ +782,5 @@ > + * of self-hosted builtins. > + * > + * Additionally, the special syntax _CallFunction(receiver, ...args, fun) > + * is supported, for which bytecode is emitted that invokes |fun| with > + * |receiver| as the this-object and ...args as the arguments.. Pre-existing nit: superfluous "." at the end @@ +902,5 @@ > static bool > +CloneValue(JSContext *cx, const Value &selfHostedValue, MutableHandleValue vp, CloneMemory &clonedObjects); > + > +static Value > +GetUnclonedValue(JSObject *selfHostedObject, jsid id) This should signal failure. Right now, it'll just return UndefinedValue, which CloneValue will then happily clone, so a failure here will just be swallowed silently. @@ +935,3 @@ > for (uint32_t i = 0; i < ids.length(); i++) { > id = ids[i]; > + Value selfHostedValue = GetUnclonedValue(selfHostedObject, id); As mentioned above, this should have error checking for GetUnclonedValue @@ +1039,1 @@ > RootedId id(cx, NameToId(name)); This should also suppress GCs. @@ +1039,2 @@ > RootedId id(cx, NameToId(name)); > + Value funVal = GetUnclonedValue(selfHostingGlobal_, id); Here, too, we need error checking. @@ +1067,2 @@ > RootedId id(cx, NameToId(name)); > + Value selfHostedValue = GetUnclonedValue(selfHostingGlobal_, id); Error checking needed.

> On top of bug 964059 this reduces an empty worker's memory > usage an additional .41 MB, to .23 MB: Excellent. I think we now have some global stuff that's shared between workers, right? How big is it, and is it being reported? > │ ├──0.07 MB (00.09%) ++ compartment(web-worker) What does that sub-tree look like? Mostly objects and shapes?

(In reply to Brian Hackett (:bhackett) from comment #2) > On top of bug 964059 this reduces an empty worker's memory > usage an additional .41 MB, to .23 MB Nice.

\o/

nominating because of https://bugzilla.mozilla.org/show_bug.cgi?id=964059#c25

Rebased patch for b2g28_v1_3t, on top of the rebased patch in bug 964059.