Bug 1524565 - Clone ScriptSourceObject just once when cloning a script with inner functions. r=tcampbell a=lizzard
authorJan de Mooij <jdemooij@mozilla.com>
Sat, 02 Feb 2019 10:42:27 +0000
changeset 515762 38126f72db7fe9f3d7098cdf73d33db800022a4d
parent 515761 16b643f92fbf41777ed213d1caab4f343ee581ec
child 515763 bf4fec8933af388b2e37a56780dc6895727a8b2d
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell, lizzard
bugs1524565
milestone66.0
Bug 1524565 - Clone ScriptSourceObject just once when cloning a script with inner functions. r=tcampbell a=lizzard Creating too many SSOs can apparently slow down the debugger and it's just wasteful. This patch passes the SSO down to the cloning functions. One nice side-effect is that the self-hosting SSO code now lives in SelfHosting.cpp instead of JSScript.cpp Differential Revision: https://phabricator.services.mozilla.com/D18347
js/src/jsapi.cpp
js/src/vm/JSFunction-inl.h
js/src/vm/JSFunction.cpp
js/src/vm/JSFunction.h
js/src/vm/JSScript.cpp
js/src/vm/JSScript.h
js/src/vm/SelfHosting.cpp
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -3249,57 +3249,54 @@ static JSObject* CloneFunctionObject(JSC
   if (!funobj->is<JSFunction>()) {
     MOZ_RELEASE_ASSERT(!IsCrossCompartmentWrapper(funobj));
     AutoRealm ar(cx, funobj);
     RootedValue v(cx, ObjectValue(*funobj));
     ReportIsNotFunction(cx, v);
     return nullptr;
   }
 
+  // Only allow cloning normal, interpreted functions.
   RootedFunction fun(cx, &funobj->as<JSFunction>());
+  if (fun->isNative() || fun->isBoundFunction() ||
+      fun->kind() != JSFunction::NormalFunction || fun->isExtended() ||
+      fun->isSelfHostedBuiltin()) {
+    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                              JSMSG_CANT_CLONE_OBJECT);
+    return nullptr;
+  }
+
   if (fun->isInterpretedLazy()) {
     AutoRealm ar(cx, fun);
     if (!JSFunction::getOrCreateScript(cx, fun)) {
       return nullptr;
     }
   }
-
-  // Only allow cloning normal, interpreted functions.
-  if (fun->isNative() || fun->isBoundFunction() ||
-      fun->kind() != JSFunction::NormalFunction || fun->isExtended()) {
-    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                              JSMSG_CANT_CLONE_OBJECT);
-    return nullptr;
-  }
+  RootedScript script(cx, fun->nonLazyScript());
 
   if (!IsFunctionCloneable(fun)) {
     JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
                               JSMSG_BAD_CLONE_FUNOBJ_SCOPE);
     return nullptr;
   }
 
   if (CanReuseScriptForClone(cx->realm(), fun, env)) {
-    // If the script is to be reused, either the script can already handle
-    // non-syntactic scopes, or there is only the standard global lexical
-    // scope.
-#ifdef DEBUG
-    // Fail here if we OOM during debug asserting.
-    // CloneFunctionReuseScript will delazify the script anyways, so we
-    // are not creating an extra failure condition for DEBUG builds.
-    if (!JSFunction::getOrCreateScript(cx, fun)) {
+    return CloneFunctionReuseScript(cx, fun, env, fun->getAllocKind());
+  }
+
+  Rooted<ScriptSourceObject*> sourceObject(cx, script->sourceObject());
+  if (cx->compartment() != sourceObject->compartment()) {
+    sourceObject = ScriptSourceObject::clone(cx, sourceObject);
+    if (!sourceObject) {
       return nullptr;
     }
-    MOZ_ASSERT(scope->as<GlobalScope>().isSyntactic() ||
-               fun->nonLazyScript()->hasNonSyntacticScope());
-#endif
-    return CloneFunctionReuseScript(cx, fun, env, fun->getAllocKind());
-  }
-
-  JSFunction* clone =
-      CloneFunctionAndScript(cx, fun, env, scope, fun->getAllocKind());
+  }
+
+  JSFunction* clone = CloneFunctionAndScript(cx, fun, env, scope, sourceObject,
+                                             fun->getAllocKind());
 
 #ifdef DEBUG
   // The cloned function should itself be cloneable.
   RootedFunction cloneRoot(cx, clone);
   MOZ_ASSERT_IF(cloneRoot, IsFunctionCloneable(cloneRoot));
 #endif
 
   return clone;
--- a/js/src/vm/JSFunction-inl.h
+++ b/js/src/vm/JSFunction-inl.h
@@ -83,17 +83,19 @@ inline JSFunction* CloneFunctionObjectIf
     return CloneFunctionReuseScript(cx, fun, parent, kind, newKind, proto);
   }
 
   RootedScript script(cx, JSFunction::getOrCreateScript(cx, fun));
   if (!script) {
     return nullptr;
   }
   RootedScope enclosingScope(cx, script->enclosingScope());
-  return CloneFunctionAndScript(cx, fun, parent, enclosingScope, kind, proto);
+  Rooted<ScriptSourceObject*> sourceObject(cx, script->sourceObject());
+  return CloneFunctionAndScript(cx, fun, parent, enclosingScope, sourceObject,
+                                kind, proto);
 }
 
 } /* namespace js */
 
 /* static */ inline JS::Result<JSFunction*, JS::OOM&> JSFunction::create(
     JSContext* cx, js::gc::AllocKind kind, js::gc::InitialHeap heap,
     js::HandleShape shape, js::HandleObjectGroup group) {
   MOZ_ASSERT(kind == js::gc::AllocKind::FUNCTION ||
--- a/js/src/vm/JSFunction.cpp
+++ b/js/src/vm/JSFunction.cpp
@@ -2282,16 +2282,17 @@ JSFunction* js::CloneFunctionReuseScript
     clone->setGroup(fun->group());
   }
   return clone;
 }
 
 JSFunction* js::CloneFunctionAndScript(JSContext* cx, HandleFunction fun,
                                        HandleObject enclosingEnv,
                                        HandleScope newScope,
+                                       Handle<ScriptSourceObject*> sourceObject,
                                        gc::AllocKind allocKind /* = FUNCTION */,
                                        HandleObject proto /* = nullptr */) {
   MOZ_ASSERT(NewFunctionEnvironmentIsWellFormed(cx, enclosingEnv));
   MOZ_ASSERT(fun->isInterpreted());
   MOZ_ASSERT(!fun->isBoundFunction());
 
   JSScript::AutoDelazify funScript(cx, fun);
   if (!funScript) {
@@ -2324,17 +2325,17 @@ JSFunction* js::CloneFunctionAndScript(J
 #endif
 
   RootedScript script(cx, fun->nonLazyScript());
   MOZ_ASSERT(script->realm() == fun->realm());
   MOZ_ASSERT(cx->compartment() == clone->compartment(),
              "Otherwise we could relazify clone below!");
 
   RootedScript clonedScript(
-      cx, CloneScriptIntoFunction(cx, newScope, clone, script));
+      cx, CloneScriptIntoFunction(cx, newScope, clone, script, sourceObject));
   if (!clonedScript) {
     return nullptr;
   }
   Debugger::onNewScript(cx, clonedScript);
 
   return clone;
 }
 
--- a/js/src/vm/JSFunction.h
+++ b/js/src/vm/JSFunction.h
@@ -967,18 +967,18 @@ extern bool CanReuseScriptForClone(JS::R
 extern JSFunction* CloneFunctionReuseScript(
     JSContext* cx, HandleFunction fun, HandleObject parent,
     gc::AllocKind kind = gc::AllocKind::FUNCTION,
     NewObjectKind newKindArg = GenericObject, HandleObject proto = nullptr);
 
 // Functions whose scripts are cloned are always given singleton types.
 extern JSFunction* CloneFunctionAndScript(
     JSContext* cx, HandleFunction fun, HandleObject parent,
-    HandleScope newScope, gc::AllocKind kind = gc::AllocKind::FUNCTION,
-    HandleObject proto = nullptr);
+    HandleScope newScope, Handle<ScriptSourceObject*> sourceObject,
+    gc::AllocKind kind = gc::AllocKind::FUNCTION, HandleObject proto = nullptr);
 
 extern JSFunction* CloneAsmJSModuleFunction(JSContext* cx, HandleFunction fun);
 
 extern JSFunction* CloneSelfHostingIntrinsic(JSContext* cx, HandleFunction fun);
 
 }  // namespace js
 
 inline js::FunctionExtended* JSFunction::toExtended() {
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -3830,19 +3830,19 @@ void js::DescribeScriptedCallerForCompil
     maybeScript.set(iter.script());
     *pcOffset = iter.pc() - maybeScript->code();
   } else {
     maybeScript.set(nullptr);
     *pcOffset = 0;
   }
 }
 
-static JSObject* CloneInnerInterpretedFunction(JSContext* cx,
-                                               HandleScope enclosingScope,
-                                               HandleFunction srcFun) {
+static JSObject* CloneInnerInterpretedFunction(
+    JSContext* cx, HandleScope enclosingScope, HandleFunction srcFun,
+    Handle<ScriptSourceObject*> sourceObject) {
   /* NB: Keep this in sync with XDRInterpretedFunction. */
   RootedObject cloneProto(cx);
   if (srcFun->isGenerator() || srcFun->isAsync()) {
     cloneProto =
         GlobalObject::getOrCreateGeneratorFunctionPrototype(cx, cx->global());
     if (!cloneProto) {
       return nullptr;
     }
@@ -3868,18 +3868,18 @@ static JSObject* CloneInnerInterpretedFu
   if (!clone) {
     return nullptr;
   }
 
   JSScript::AutoDelazify srcScript(cx, srcFun);
   if (!srcScript) {
     return nullptr;
   }
-  JSScript* cloneScript =
-      CloneScriptIntoFunction(cx, enclosingScope, clone, srcScript);
+  JSScript* cloneScript = CloneScriptIntoFunction(cx, enclosingScope, clone,
+                                                  srcScript, sourceObject);
   if (!cloneScript) {
     return nullptr;
   }
 
   if (!JSFunction::setTypeForScriptedFunction(cx, clone)) {
     return nullptr;
   }
 
@@ -3971,16 +3971,17 @@ bool js::detail::CopyScript(JSContext* c
   }
 
   /* Objects */
 
   AutoObjectVector objects(cx);
   if (nobjects != 0) {
     RootedObject obj(cx);
     RootedObject clone(cx);
+    Rooted<ScriptSourceObject*> sourceObject(cx, dst->sourceObject());
     for (const GCPtrObject& elem : src->objects()) {
       obj = elem.get();
       clone = nullptr;
       if (obj->is<RegExpObject>()) {
         clone = CloneScriptRegExpObject(cx, obj->as<RegExpObject>());
       } else if (obj->is<JSFunction>()) {
         RootedFunction innerFun(cx, &obj->as<JSFunction>());
         if (innerFun->isNative()) {
@@ -3997,17 +3998,18 @@ bool js::detail::CopyScript(JSContext* c
             if (!JSFunction::getOrCreateScript(cx, innerFun)) {
               return false;
             }
           }
 
           Scope* enclosing = innerFun->nonLazyScript()->enclosingScope();
           RootedScope enclosingClone(cx,
                                      scopes[FindScopeIndex(src, *enclosing)]);
-          clone = CloneInnerInterpretedFunction(cx, enclosingClone, innerFun);
+          clone = CloneInnerInterpretedFunction(cx, enclosingClone, innerFun,
+                                                sourceObject);
         }
       } else {
         clone = DeepCloneObjectLiteral(cx, obj, TenuredObject);
       }
 
       if (!clone || !objects.append(clone)) {
         return false;
       }
@@ -4063,61 +4065,46 @@ bool js::detail::CopyScript(JSContext* c
     for (unsigned i = 0; i < nobjects; ++i) {
       array[i].init(objects[i]);
     }
   }
 
   return true;
 }
 
-static JSScript* CreateEmptyScriptForClone(JSContext* cx, HandleScript src) {
-  /*
-   * Wrap the script source object as needed. Self-hosted scripts may be
-   * in another runtime, so lazily create a new script source object to
-   * use for them.
-   */
-  RootedScriptSourceObject sourceObject(cx);
-  if (src->realm()->isSelfHostingRealm()) {
-    if (!cx->realm()->selfHostingScriptSource) {
-      CompileOptions options(cx);
-      FillSelfHostingCompileOptions(options);
-
-      ScriptSourceObject* obj = frontend::CreateScriptSourceObject(cx, options);
-      if (!obj) {
-        return nullptr;
-      }
-      cx->realm()->selfHostingScriptSource.set(obj);
-    }
-    sourceObject = cx->realm()->selfHostingScriptSource;
-  } else {
-    sourceObject = src->sourceObject();
-    if (cx->compartment() != sourceObject->compartment()) {
-      sourceObject = ScriptSourceObject::clone(cx, sourceObject);
-      if (!sourceObject) {
-        return nullptr;
-      }
-    }
-  }
+static JSScript* CreateEmptyScriptForClone(
+    JSContext* cx, HandleScript src, Handle<ScriptSourceObject*> sourceObject) {
+  MOZ_ASSERT(cx->compartment() == sourceObject->compartment());
+  MOZ_ASSERT_IF(src->realm()->isSelfHostingRealm(),
+                sourceObject == cx->realm()->selfHostingScriptSource);
 
   CompileOptions options(cx);
   options.setMutedErrors(src->mutedErrors())
       .setSelfHostingMode(src->selfHosted())
       .setNoScriptRval(src->noScriptRval());
 
   return JSScript::Create(cx, options, sourceObject, src->sourceStart(),
                           src->sourceEnd(), src->toStringStart(),
                           src->toStringEnd());
 }
 
 JSScript* js::CloneGlobalScript(JSContext* cx, ScopeKind scopeKind,
                                 HandleScript src) {
   MOZ_ASSERT(scopeKind == ScopeKind::Global ||
              scopeKind == ScopeKind::NonSyntactic);
 
-  RootedScript dst(cx, CreateEmptyScriptForClone(cx, src));
+  Rooted<ScriptSourceObject*> sourceObject(cx, src->sourceObject());
+  if (cx->compartment() != sourceObject->compartment()) {
+    sourceObject = ScriptSourceObject::clone(cx, sourceObject);
+    if (!sourceObject) {
+      return nullptr;
+    }
+  }
+
+  RootedScript dst(cx, CreateEmptyScriptForClone(cx, src, sourceObject));
   if (!dst) {
     return nullptr;
   }
 
   MOZ_ASSERT(src->bodyScopeIndex() == 0);
   Rooted<GCVector<Scope*>> scopes(cx, GCVector<Scope*>(cx));
   Rooted<GlobalScope*> original(cx, &src->bodyScope()->as<GlobalScope>());
   GlobalScope* clone = GlobalScope::clone(cx, original, scopeKind);
@@ -4127,22 +4114,23 @@ JSScript* js::CloneGlobalScript(JSContex
 
   if (!detail::CopyScript(cx, src, dst, &scopes)) {
     return nullptr;
   }
 
   return dst;
 }
 
-JSScript* js::CloneScriptIntoFunction(JSContext* cx, HandleScope enclosingScope,
-                                      HandleFunction fun, HandleScript src) {
+JSScript* js::CloneScriptIntoFunction(
+    JSContext* cx, HandleScope enclosingScope, HandleFunction fun,
+    HandleScript src, Handle<ScriptSourceObject*> sourceObject) {
   MOZ_ASSERT(fun->isInterpreted());
   MOZ_ASSERT(!fun->hasScript() || fun->hasUncompletedScript());
 
-  RootedScript dst(cx, CreateEmptyScriptForClone(cx, src));
+  RootedScript dst(cx, CreateEmptyScriptForClone(cx, src, sourceObject));
   if (!dst) {
     return nullptr;
   }
 
   // Clone the non-intra-body scopes.
   Rooted<GCVector<Scope*>> scopes(cx, GCVector<Scope*>(cx));
   RootedScope original(cx);
   RootedScope enclosingClone(cx);
--- a/js/src/vm/JSScript.h
+++ b/js/src/vm/JSScript.h
@@ -1122,22 +1122,26 @@ class ScriptSourceHolder {
   ScriptSource* get() const { return ss; }
 };
 
 // [SMDOC] ScriptSourceObject
 //
 // ScriptSourceObject stores the ScriptSource and GC pointers related to it.
 //
 // ScriptSourceObjects can be cloned when we clone the JSScript (in order to
-// execute the script in a different realm/compartment). In this case we create
-// a new SSO that stores (a wrapper for) the original SSO in its "canonical
-// slot". The canonical SSO is always used for the private, introductionScript,
+// execute the script in a different compartment). In this case we create a new
+// SSO that stores (a wrapper for) the original SSO in its "canonical slot".
+// The canonical SSO is always used for the private, introductionScript,
 // element, elementAttributeName slots. This means their accessors may return an
 // object in a different compartment, hence the "unwrapped" prefix.
 //
+// Note that we don't clone the SSO when cloning the script for a different
+// realm in the same compartment, so sso->realm() does not necessarily match the
+// script's realm.
+//
 // We need ScriptSourceObject (instead of storing these GC pointers in the
 // ScriptSource itself) to properly account for cross-zone pointers: the
 // canonical SSO will be stored in the wrapper map if necessary so GC will do
 // the right thing.
 class ScriptSourceObject : public NativeObject {
   static const ClassOps classOps_;
 
   static ScriptSourceObject* createInternal(JSContext* cx, ScriptSource* source,
@@ -3181,17 +3185,18 @@ extern void DescribeScriptedCallerForCom
  * Like DescribeScriptedCallerForCompilation, but this function avoids looking
  * up the script/pc and the full linear scan to compute line number.
  */
 extern void DescribeScriptedCallerForDirectEval(
     JSContext* cx, HandleScript script, jsbytecode* pc, const char** file,
     unsigned* linenop, uint32_t* pcOffset, bool* mutedErrors);
 
 JSScript* CloneScriptIntoFunction(JSContext* cx, HandleScope enclosingScope,
-                                  HandleFunction fun, HandleScript src);
+                                  HandleFunction fun, HandleScript src,
+                                  Handle<ScriptSourceObject*> sourceObject);
 
 JSScript* CloneGlobalScript(JSContext* cx, ScopeKind scopeKind,
                             HandleScript src);
 
 } /* namespace js */
 
 // JS::ubi::Nodes can point to js::LazyScripts; they're js::gc::Cell instances
 // with no associated compartment.
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -3127,16 +3127,36 @@ static JSString* CloneString(JSContext* 
   }
 
   return chars.isLatin1()
              ? NewStringCopyN<CanGC>(cx, chars.latin1Range().begin().get(), len)
              : NewStringCopyNDontDeflate<CanGC>(
                    cx, chars.twoByteRange().begin().get(), len);
 }
 
+// Returns the ScriptSourceObject to use for cloned self-hosted scripts in the
+// current realm.
+static ScriptSourceObject* SelfHostingScriptSourceObject(JSContext* cx) {
+  if (ScriptSourceObject* sso = cx->realm()->selfHostingScriptSource) {
+    return sso;
+  }
+
+  CompileOptions options(cx);
+  FillSelfHostingCompileOptions(options);
+
+  ScriptSourceObject* sourceObject =
+      frontend::CreateScriptSourceObject(cx, options);
+  if (!sourceObject) {
+    return nullptr;
+  }
+
+  cx->realm()->selfHostingScriptSource.set(sourceObject);
+  return sourceObject;
+}
+
 static JSObject* CloneObject(JSContext* cx,
                              HandleNativeObject selfHostedObject) {
 #ifdef DEBUG
   // Object hash identities are owned by the hashed object, which may be on a
   // different thread than the clone target. In theory, these objects are all
   // tenured and will not be compacted; however, we simply avoid the issue
   // altogether by skipping the cycle-detection when off thread.
   mozilla::Maybe<AutoCycleDetector> detect;
@@ -3162,20 +3182,25 @@ static JSObject* CloneObject(JSContext* 
       MOZ_ASSERT(!selfHostedFunction->isArrow());
       js::gc::AllocKind kind = hasName ? gc::AllocKind::FUNCTION_EXTENDED
                                        : selfHostedFunction->getAllocKind();
 
       Handle<GlobalObject*> global = cx->global();
       Rooted<LexicalEnvironmentObject*> globalLexical(
           cx, &global->lexicalEnvironment());
       RootedScope emptyGlobalScope(cx, &global->emptyGlobalScope());
+      Rooted<ScriptSourceObject*> sourceObject(
+          cx, SelfHostingScriptSourceObject(cx));
+      if (!sourceObject) {
+        return nullptr;
+      }
       MOZ_ASSERT(
           !CanReuseScriptForClone(cx->realm(), selfHostedFunction, global));
       clone = CloneFunctionAndScript(cx, selfHostedFunction, globalLexical,
-                                     emptyGlobalScope, kind);
+                                     emptyGlobalScope, sourceObject, kind);
       // To be able to re-lazify the cloned function, its name in the
       // self-hosting compartment has to be stored on the clone.
       if (clone && hasName) {
         Value nameVal = StringValue(selfHostedFunction->explicitName());
         clone->as<JSFunction>().setExtendedSlot(LAZY_FUNCTION_NAME_SLOT,
                                                 nameVal);
       }
     } else {
@@ -3305,25 +3330,32 @@ bool JSRuntime::cloneSelfHostedFunctionS
   MOZ_ASSERT(targetFun->isInterpretedLazy());
   MOZ_ASSERT(targetFun->isSelfHostedBuiltin());
 
   RootedScript sourceScript(cx, JSFunction::getOrCreateScript(cx, sourceFun));
   if (!sourceScript) {
     return false;
   }
 
+  Rooted<ScriptSourceObject*> sourceObject(cx,
+                                           SelfHostingScriptSourceObject(cx));
+  if (!sourceObject) {
+    return false;
+  }
+
   // Assert that there are no intervening scopes between the global scope
   // and the self-hosted script. Toplevel lexicals are explicitly forbidden
   // by the parser when parsing self-hosted code. The fact they have the
   // global lexical scope on the scope chain is for uniformity and engine
   // invariants.
   MOZ_ASSERT(sourceScript->outermostScope()->enclosing()->kind() ==
              ScopeKind::Global);
   RootedScope emptyGlobalScope(cx, &cx->global()->emptyGlobalScope());
-  if (!CloneScriptIntoFunction(cx, emptyGlobalScope, targetFun, sourceScript)) {
+  if (!CloneScriptIntoFunction(cx, emptyGlobalScope, targetFun, sourceScript,
+                               sourceObject)) {
     return false;
   }
   MOZ_ASSERT(!targetFun->isInterpretedLazy());
 
   MOZ_ASSERT(sourceFun->nargs() == targetFun->nargs());
   MOZ_ASSERT(sourceScript->hasRest() == targetFun->nonLazyScript()->hasRest());
   MOZ_ASSERT(targetFun->strict(), "Self-hosted builtins must be strict");