Bug 1447362: Avoid Atomize calls when binding a bound function. r=jandem

js/src/jit/CodeGenerator.cpp

@@ -13355,9 +13355,9 @@ CodeGenerator::visitFinishBoundFunctionInit(LFinishBoundFunctionInit* lir)
     Label notBoundTarget, loadName;
     masm.branchTest32(Assembler::Zero, temp1, Imm32(JSFunction::BOUND_FUN), &notBoundTarget);
     {
-        // Target's name atom doesn't contain the bound function prefix, so we
-        // need to call into the VM.
-        masm.branchTest32(Assembler::Zero, temp1,
+        // Call into the VM if the target's name atom contains the bound
+        // function prefix.
+        masm.branchTest32(Assembler::NonZero, temp1,
                           Imm32(JSFunction::HAS_BOUND_FUNCTION_NAME_PREFIX), slowPath);
 
         // We also take the slow path when target's length isn't an int32.

js/src/vm/JSFunction.cpp

@@ -60,6 +60,7 @@ using namespace js::gc;
 using namespace js::frontend;
 
 using mozilla::ArrayLength;
+using mozilla::CheckedInt;
 using mozilla::Maybe;
 using mozilla::Some;
 
@@ -537,7 +538,7 @@ fun_resolve(JSContext* cx, HandleObject obj, HandleId id, bool* resolvedp)
             if (fun->hasResolvedName())
                 return true;
 
-            RootedAtom name(cx);
+            RootedString name(cx);
             if (!JSFunction::getUnresolvedName(cx, fun, &name))
                 return false;
 
@@ -1032,12 +1033,15 @@ js::FunctionToString(JSContext* cx, HandleFunction fun, bool isToSource)
         if (fun->explicitName()) {
             if (!out.append(' '))
                 return nullptr;
-            if (fun->isBoundFunction() && !fun->hasBoundFunctionNamePrefix()) {
-                if (!out.append(cx->names().boundWithSpace))
+
+            if (fun->isBoundFunction()) {
+                JSLinearString* boundName = JSFunction::getBoundFunctionName(cx, fun);
+                if (!boundName || !out.append(boundName))
+                    return nullptr;
+            } else {
+                if (!out.append(fun->explicitName()))
                     return nullptr;
             }
-            if (!out.append(fun->explicitName()))
-                return nullptr;
         }
 
         if (fun->isInterpreted() &&
@@ -1321,41 +1325,90 @@ JSFunction::getUnresolvedLength(JSContext* cx, HandleFunction fun, MutableHandle
     return true;
 }
 
-/* static */ bool
-JSFunction::getUnresolvedName(JSContext* cx, HandleFunction fun, MutableHandleAtom v)
+JSAtom*
+JSFunction::infallibleGetUnresolvedName(JSContext* cx)
 {
-    MOZ_ASSERT(!IsInternalFunctionObject(*fun));
-    MOZ_ASSERT(!fun->hasResolvedName());
+    MOZ_ASSERT(!IsInternalFunctionObject(*this));
+    MOZ_ASSERT(!hasResolvedName());
 
-    JSAtom* name = fun->explicitOrInferredName();
-    if (fun->isClassConstructor()) {
-        // Unnamed class expressions should not get a .name property at all.
-        if (name)
-            v.set(name);
+    if (JSAtom* name = explicitOrInferredName())
+        return name;
+
+    // Unnamed class expressions should not get a .name property at all.
+    if (isClassConstructor())
+        return nullptr;
+
+    return cx->names().empty;
+}
+
+/* static */ bool
+JSFunction::getUnresolvedName(JSContext* cx, HandleFunction fun, MutableHandleString v)
+{
+    if (fun->isBoundFunction()) {
+        JSLinearString* name = JSFunction::getBoundFunctionName(cx, fun);
+        if (!name)
+            return false;
+
+        v.set(name);
         return true;
     }
 
-    if (fun->isBoundFunction() && !fun->hasBoundFunctionNamePrefix()) {
-        // Bound functions are never unnamed.
-        MOZ_ASSERT(name);
+    v.set(fun->infallibleGetUnresolvedName(cx));
+    return true;
+}
 
-        if (name->length() > 0) {
-            StringBuffer sb(cx);
-            if (!sb.append(cx->names().boundWithSpace) || !sb.append(name))
-                return false;
+/* static */ JSLinearString*
+JSFunction::getBoundFunctionName(JSContext* cx, HandleFunction fun)
+{
+    MOZ_ASSERT(fun->isBoundFunction());
+    JSAtom* name = fun->explicitName();
 
-            name = sb.finishAtom();
-            if (!name)
-                return false;
-        } else {
-            name = cx->names().boundWithSpace;
-        }
+    // Bound functions are never unnamed.
+    MOZ_ASSERT(name);
 
-        fun->setPrefixedBoundFunctionName(name);
+    // If the bound function prefix is present, return the name as is.
+    if (fun->hasBoundFunctionNamePrefix())
+        return name;
+
+    // Otherwise return "bound " * (number of bound function targets) + name.
+    size_t boundTargets = 0;
+    for (JSFunction* boundFn = fun; boundFn->isBoundFunction(); ) {
+        boundTargets++;
+
+        JSObject* target = boundFn->getBoundFunctionTarget();
+        if (!target->is<JSFunction>())
+            break;
+        boundFn = &target->as<JSFunction>();
     }
 
-    v.set(name != nullptr ? name : cx->names().empty);
-    return true;
+    // |function /*unnamed*/ (){...}.bind()| is a common case, handle it here.
+    if (name->empty() && boundTargets == 1)
+        return cx->names().boundWithSpace;
+
+    static constexpr char boundWithSpaceChars[] = "bound ";
+    static constexpr size_t boundWithSpaceCharsLength =
+        ArrayLength(boundWithSpaceChars) - 1; // No trailing '\0'.
+    MOZ_ASSERT(StringEqualsAscii(cx->names().boundWithSpace, boundWithSpaceChars));
+
+    StringBuffer sb(cx);
+    if (name->hasTwoByteChars() && !sb.ensureTwoByteChars())
+        return nullptr;
+
+    CheckedInt<size_t> len(boundTargets);
+    len *= boundWithSpaceCharsLength;
+    len += name->length();
+    if (!len.isValid()) {
+        ReportAllocationOverflow(cx);
+        return nullptr;
+    }
+    if (!sb.reserve(len.value()))
+        return nullptr;
+
+    while (boundTargets--)
+        sb.infallibleAppend(boundWithSpaceChars, boundWithSpaceCharsLength);
+    sb.infallibleAppendSubstring(name, 0, name->length());
+
+    return sb.finishString();
 }
 
 static const js::Value&
@@ -1401,6 +1454,18 @@ JSFunction::getBoundFunctionArgumentCount() const
     return GetBoundFunctionArguments(this)->length();
 }
 
+static JSAtom*
+AppendBoundFunctionPrefix(JSContext* cx, JSString* str)
+{
+    static constexpr char boundWithSpaceChars[] = "bound ";
+    MOZ_ASSERT(StringEqualsAscii(cx->names().boundWithSpace, boundWithSpaceChars));
+
+    StringBuffer sb(cx);
+    if (!sb.append(boundWithSpaceChars) || !sb.append(str))
+        return nullptr;
+    return sb.finishAtom();
+}
+
 /* static */ bool
 JSFunction::finishBoundFunctionInit(JSContext* cx, HandleFunction bound, HandleObject targetObj,
                                     int32_t argCount)
@@ -1457,11 +1522,26 @@ JSFunction::finishBoundFunctionInit(JSContext* cx, HandleFunction bound, HandleO
     // 19.2.3.2 Function.prototype.bind, step 8.
     bound->setExtendedSlot(BOUND_FUN_LENGTH_SLOT, NumberValue(length));
 
+    MOZ_ASSERT(!bound->hasGuessedAtom());
+
     // Try to avoid invoking the resolve hook.
-    RootedAtom name(cx);
+    JSAtom* name = nullptr;
     if (targetObj->is<JSFunction>() && !targetObj->as<JSFunction>().hasResolvedName()) {
-        if (!JSFunction::getUnresolvedName(cx, targetObj.as<JSFunction>(), &name))
-            return false;
+        JSFunction* targetFn = &targetObj->as<JSFunction>();
+
+        // If the target is a bound function with a prefixed name, we can't
+        // lazily compute the full name in getBoundFunctionName(), therefore
+        // we need to append the bound function name prefix here.
+        if (targetFn->isBoundFunction() && targetFn->hasBoundFunctionNamePrefix()) {
+            name = AppendBoundFunctionPrefix(cx, targetFn->explicitName());
+            if (!name)
+                return false;
+            bound->setPrefixedBoundFunctionName(name);
+        } else {
+            name = targetFn->infallibleGetUnresolvedName(cx);
+            if (name)
+                bound->setAtom(name);
+        }
     }
 
     // 19.2.3.2 Function.prototype.bind, steps 9-11.
@@ -1472,18 +1552,26 @@ JSFunction::finishBoundFunctionInit(JSContext* cx, HandleFunction bound, HandleO
             return false;
 
         // 19.2.3.2 Function.prototype.bind, step 10.
-        if (targetName.isString() && !targetName.toString()->empty()) {
+        if (!targetName.isString())
+            targetName.setString(cx->names().empty);
+
+        // If the target itself is a bound function (with a resolved name), we
+        // can't compute the full name in getBoundFunctionName() based only on
+        // the number of bound target functions, therefore we need to store
+        // the complete prefixed name here.
+        if (targetObj->is<JSFunction>() && targetObj->as<JSFunction>().isBoundFunction()) {
+            name = AppendBoundFunctionPrefix(cx, targetName.toString());
+            if (!name)
+                return false;
+            bound->setPrefixedBoundFunctionName(name);
+        } else {
             name = AtomizeString(cx, targetName.toString());
             if (!name)
                 return false;
-        } else {
-            name = cx->names().empty;
+            bound->setAtom(name);
         }
     }
 
-    MOZ_ASSERT(!bound->hasGuessedAtom());
-    bound->setAtom(name);
-
     return true;
 }
 

js/src/vm/JSFunction.h

@@ -341,8 +341,12 @@ class JSFunction : public js::NativeObject
     static bool getUnresolvedLength(JSContext* cx, js::HandleFunction fun,
                                     js::MutableHandleValue v);
 
+    JSAtom* infallibleGetUnresolvedName(JSContext* cx);
+
     static bool getUnresolvedName(JSContext* cx, js::HandleFunction fun,
-                                  js::MutableHandleAtom v);
+                                  js::MutableHandleString v);
+
+    static JSLinearString* getBoundFunctionName(JSContext* cx, js::HandleFunction fun);
 
     JSAtom* explicitName() const {
         return (hasInferredName() || hasGuessedAtom()) ? nullptr : atom_.get();