Currently we initialize the SH global using GlobalObject::initStandardClasses(). But since it would be unsafe for SH code to actually use the mutable standard bindings, the first thing Utilities.js does is extract the bindings that we actually intend to use, and store those functions in SH-private variables. I propose *not* using initStandardClasses on the SH global. Instead, we can turn the first part of Utilities.js into a C++ table of names and JSNative pointers, and initialize the SH global directly from that. Then the SH global would not have normal builtins on it at all. This would have the side effect of fixing bug 1062907.

I like this. Makes self-hosted JS even less like normal JS, but then again that's clearly been the case for a long time.

In the EIBTI spirit, this is wonderful!

(In reply to Boris Zbarsky [:bz] from comment #2) > In the EIBTI spirit, this is wonderful! Yes! As I said on IRC earlier, this is very obviously a good idea. Irritatingly obvious, really: we should've done this a long time ago.

This seems to work quite nicely. For the most part, I had to extend the intrinsic_functions JSFunctionSpec[] to include all the std_Foo functions. Some of those had to be exported in their builtins' headers. There are a few gnarly issues around iterators, as one would expect. Also, A few builtins have to at least *exist*, notably Function, Object, and Array. The first two are handled automatically, Array I had to do manually. Seems okay-ish to me, though. Try-servering here: https://tbpl.mozilla.org/?tree=Try&rev=de5a3910023d https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=de5a3910023d

Because the alphabet is hard

Now even without PJS bustage. I had to install a few builtin constructor functions after all: Array is just flat out needed for, oh, about everything. Uint{8,32}Array are needed for some PJS functions. To not have them install unnecessary cruft, I introduced a function InitBareBuiltinCtor that does what you think it does. Now really does pass all (not let-busted) tests locally, and I'm very optimistic about the new try run here: https://tbpl.mozilla.org/?tree=Try&rev=55542cdb6937 https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=55542cdb6937

Comment on attachment 8492626 [details] [diff] [review] Only install a small subset of the standard builtins in the self-hosting global. v3 Review of attachment 8492626 [details] [diff] [review]: ----------------------------------------------------------------- I'm not sure this simplifies things as much as you'd hoped, but it'll solve the startup perf issue. r=me with a nit or two. ::: js/src/builtin/MapObject.h @@ +93,5 @@ > static const Class class_; > > // Entries is every key followed by value. > static bool entries(JSContext *cx, HandleObject obj, JS::AutoValueVector *entries); > + static bool entries(JSContext *cx, unsigned argc, Value *vp); Since these two methods are so different, please rename the first one here from entries() to getKeysAndValuesInterleaved(). Then the comment can be deleted. I think it's only called by vm/StructuredClone.cpp. ::: js/src/jsobj.cpp @@ +157,5 @@ > /* > * Define self-hosted functions after setting the intrinsics holder > * (which is needed to define self-hosted functions) > */ > + if (!isSelfHostingGlobal && !JS_DefineFunctions(cx, ctor, object_static_selfhosted_methods)) See "style micro-nit" below. ::: js/src/jsweakmap.cpp @@ +588,5 @@ > > if (!LinkConstructorAndPrototype(cx, ctor, weakMapProto)) > return nullptr; > > + if (defineMembers && !DefinePropertiesAndFunctions(cx, weakMapProto, nullptr, weak_map_methods)) Style micro-nit: I like this style a little better: if (defineMembers) { if (!DefinePropertiesAndFunctions...) return false; } because it separates "should I do this?" questions from "did that fail?" questions. To me it makes control flow clearer. djvj at least agrees, but this is totally your call. ::: js/src/vm/GlobalObject.cpp @@ +300,5 @@ > + RootedObject ctor(cx, clasp->spec.createConstructor(cx, protoKey)); > + if (!ctor) > + return false; > + > + return GlobalObject::initBuiltinConstructor(cx, global, protoKey, ctor, proto); Wow, these are some weird objects we're creating. I find myself wishing there were a way the prototype object could just not exist... @@ +303,5 @@ > + > + return GlobalObject::initBuiltinConstructor(cx, global, protoKey, ctor, proto); > +} > + > + Style nit: House style bans double blank lines. @@ +331,5 @@ > + return false; > + > + return InitBareWeakMapCtor(cx, global) && > + initStopIterationClass(cx, global) && > + InitSelfHostingCollectionIteratorFunctions(cx, global); It would be OK to consolidate: return InitBareBuiltinCtor(cx, global, JSProto_Array) && InitBareBuiltinCtor(cx, global, JSProto_Uint8Array) && InitBareBuiltinCtor(cx, global, JSProto_Uint32Array) && JS_DefineFunctions(cx, global, builtins) && InitBareWeakMapCtor(cx, global) && ...;

Thanks for the review! remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/ba00c79e932c (In reply to Jason Orendorff [:jorendorff] from comment #7) > I'm not sure this simplifies things as much as you'd hoped, but it'll solve > the startup perf issue. r=me with a nit or two. Oh, the initialization itself certainly didn't get simpler. I didn't expect it to, though: it's working with self-hosted code that gets easier with this. Fewer footguns and all that. See the updates I just made to the documentation for proof: https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Internals/self-hosting$compare?to=674201&from=586443 > because it separates "should I do this?" questions from "did that fail?" > questions. To me it makes control flow clearer. djvj at least agrees, but > this is totally your call. I agree, that is better. > > + return GlobalObject::initBuiltinConstructor(cx, global, protoKey, ctor, proto); > > Wow, these are some weird objects we're creating. I find myself wishing > there were a way the prototype object could just not exist... Yeah :( At least they're mostly harmless. > It would be OK to consolidate: > > return InitBareBuiltinCtor(cx, global, JSProto_Array) && > InitBareBuiltinCtor(cx, global, JSProto_Uint8Array) && > InitBareBuiltinCtor(cx, global, JSProto_Uint32Array) && > JS_DefineFunctions(cx, global, builtins) && > InitBareWeakMapCtor(cx, global) && > ...; Good call. I consolidated and reordered things to have the order make some modicum of sense.