Issue #1952 - m-c 1365387: Optimize FinishBoundFunctionInit by delaying atomizing the function name

3 files changed, 59 insertions, 40 deletions

diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp
index be1d39cd19..0d0a899b03 100644
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -339,11 +339,11 @@ CallerSetterImpl(JSContext* cx, const CallArgs& args)
 {
     MOZ_ASSERT(IsFunction(args.thisv()));
 
-    // We just have to return |undefined|, but first we call CallerGetterImpl

-    // because we need the same strict-mode and security checks.

-

+    // We just have to return |undefined|, but first we call CallerGetterImpl
+    // because we need the same strict-mode and security checks.
+
     if (!CallerGetterImpl(cx, args)) {
-        return false;

+        return false;
     }
 
     args.rval().setUndefined();
@@ -498,9 +498,12 @@ fun_resolve(JSContext* cx, HandleObject obj, HandleId id, bool* resolvedp)
             if (fun->hasResolvedName())
                 return true;
 
+            RootedAtom name(cx);
+            if (!JSFunction::getUnresolvedName(cx, fun, &name))
+                return false;
+
             // Don't define an own .name property for unnamed functions.
-            JSAtom* name = fun->getUnresolvedName(cx);
-            if (name == nullptr)
+            if (!name)
                 return true;
 
             v.setString(name);
@@ -997,7 +1000,7 @@ js::FunctionToString(JSContext* cx, HandleFunction fun, bool prettyPrint)
         return nullptr;
     }
 
-    auto AppendPrelude = [&out, &fun]() {
+    auto AppendPrelude = [cx, &out, &fun]() {
         if (fun->isAsync()) {
             if (!out.append("async "))
                 return false;
@@ -1016,6 +1019,10 @@ js::FunctionToString(JSContext* cx, HandleFunction fun, bool prettyPrint)
         if (fun->explicitName()) {
             if (!out.append(' '))
                 return false;
+            if (fun->isBoundFunction()) {
+                if (!out.append(cx->names().boundWithSpace))
+                    return false;
+            }
             if (!out.append(fun->explicitName()))
                 return false;
         }
@@ -1327,23 +1334,42 @@ JSFunction::getUnresolvedLength(JSContext* cx, HandleFunction fun, MutableHandle
     return true;
 }
 
-JSAtom*
-JSFunction::getUnresolvedName(JSContext* cx)
+/* static */ bool
+JSFunction::getUnresolvedName(JSContext* cx, HandleFunction fun, MutableHandleAtom v)
 {
-    MOZ_ASSERT(!IsInternalFunctionObject(*this));
-    MOZ_ASSERT(!hasResolvedName());
+    MOZ_ASSERT(!IsInternalFunctionObject(*fun));
+    MOZ_ASSERT(!fun->hasResolvedName());
 
-    if (isClassConstructor()) {
+    JSAtom* name = fun->explicitOrCompileTimeName();
+    if (fun->isClassConstructor()) {
         // It's impossible to have an empty named class expression. We use
         // empty as a sentinel when creating default class constructors.
-        MOZ_ASSERT(explicitOrCompileTimeName() != cx->names().empty);
+        MOZ_ASSERT(name != cx->names().empty);
 
         // Unnamed class expressions should not get a .name property at all.
-        return explicitOrCompileTimeName();
+        if (name)
+            v.set(name);
+        return true;
     }
 
-    return explicitOrCompileTimeName() != nullptr ? explicitOrCompileTimeName()
-                                                  : cx->names().empty;
+    if (fun->isBoundFunction()) {
+        // Bound functions are never unnamed.
+        MOZ_ASSERT(name);
+
+        StringBuffer sb(cx);
+        if (!sb.append(cx->names().boundWithSpace) || !sb.append(name))
+            return false;
+
+        JSAtom* boundName = sb.finishAtom();
+        if (!boundName)
+            return false;
+
+        v.set(boundName);
+        return true;
+    }
+
+    v.set(name != nullptr ? name : cx->names().empty);
+    return true;
 }
 
 static const js::Value&
diff --git a/js/src/jsfun.h b/js/src/jsfun.h
index 38cc832800..2bc846b07a 100644
--- a/js/src/jsfun.h
+++ b/js/src/jsfun.h
@@ -310,7 +310,8 @@ class JSFunction : public js::NativeObject
     static bool getUnresolvedLength(JSContext* cx, js::HandleFunction fun,
                                     js::MutableHandleValue v);
 
-    JSAtom* getUnresolvedName(JSContext* cx);
+    static bool getUnresolvedName(JSContext* cx, js::HandleFunction fun,
+                                  js::MutableHandleAtom v);
 
     JSAtom* explicitName() const {
         return (hasCompileTimeName() || hasGuessedAtom()) ? nullptr : atom_.get();
diff --git a/js/src/vm/SelfHosting.cpp b/js/src/vm/SelfHosting.cpp
index 38785a822c..29c75c2064 100644
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -571,39 +571,31 @@ intrinsic_FinishBoundFunctionInit(JSContext* cx, unsigned argc, Value* vp)
     bound->setExtendedSlot(BOUND_FUN_LENGTH_SLOT, NumberValue(length));
 
     // Try to avoid invoking the resolve hook.
-    JSAtom* name = nullptr;
-    if (targetObj->is<JSFunction>() && !targetObj->as<JSFunction>().hasResolvedName())
-        name = targetObj->as<JSFunction>().getUnresolvedName(cx);
+    RootedAtom name(cx);
+    if (targetObj->is<JSFunction>() && !targetObj->as<JSFunction>().hasResolvedName()) {
+        if (!JSFunction::getUnresolvedName(cx, targetObj.as<JSFunction>(), &name))
+            return false;
+    }
 
-    RootedString rootedName(cx);
-    if (name) {
-        rootedName = name;
-    } else {
+    // 19.2.3.2 Function.prototype.bind, steps 9-11.
+    if (!name) {
         // 19.2.3.2 Function.prototype.bind, step 9.
         RootedValue targetName(cx);
         if (!GetProperty(cx, targetObj, targetObj, cx->names().name, &targetName))
             return false;
 
         // 19.2.3.2 Function.prototype.bind, step 10.
-        if (targetName.isString())
-            rootedName = targetName.toString();
+        if (targetName.isString() && !targetName.toString()->empty()) {
+            name = AtomizeString(cx, targetName.toString());
+            if (!name)
+                return false;
+        } else {
+            name = cx->names().empty;
+        }
     }
 
-    // 19.2.3.2 Function.prototype.bind, step 11 (Inlined SetFunctionName).
     MOZ_ASSERT(!bound->hasGuessedAtom());
-    if (rootedName && !rootedName->empty()) {
-        StringBuffer sb(cx);
-        if (!sb.append(cx->names().boundWithSpace) || !sb.append(rootedName))
-            return false;
-
-        RootedAtom nameAtom(cx, sb.finishAtom());
-        if (!nameAtom)
-            return false;
-
-        bound->setAtom(nameAtom);
-    } else {
-        bound->setAtom(cx->names().boundWithSpace);
-    }
+    bound->setAtom(name);
 
     args.rval().setUndefined();
     return true;