Issue #1952 - m-c 1371593: Avoid repeated string atomizations when retrieving the unresolved name of a bound function

2 files changed, 40 insertions, 14 deletions

diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp
index 0d0a899b03..c409a0f29b 100644
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -1019,7 +1019,7 @@ js::FunctionToString(JSContext* cx, HandleFunction fun, bool prettyPrint)
         if (fun->explicitName()) {
             if (!out.append(' '))
                 return false;
-            if (fun->isBoundFunction()) {
+            if (fun->isBoundFunction() && !fun->hasBoundFunctionNamePrefix()) {
                 if (!out.append(cx->names().boundWithSpace))
                     return false;
             }
@@ -1352,20 +1352,23 @@ JSFunction::getUnresolvedName(JSContext* cx, HandleFunction fun, MutableHandleAt
         return true;
     }
 
-    if (fun->isBoundFunction()) {
+    if (fun->isBoundFunction() && !fun->hasBoundFunctionNamePrefix()) {
         // Bound functions are never unnamed.
         MOZ_ASSERT(name);
 
-        StringBuffer sb(cx);
-        if (!sb.append(cx->names().boundWithSpace) || !sb.append(name))
-            return false;
+        if (name->length() > 0) {
+            StringBuffer sb(cx);
+            if (!sb.append(cx->names().boundWithSpace) || !sb.append(name))
+                return false;
 
-        JSAtom* boundName = sb.finishAtom();
-        if (!boundName)
-            return false;
+            name = sb.finishAtom();
+            if (!name)
+                return false;
+        } else {
+            name = cx->names().boundWithSpace;
+        }
 
-        v.set(boundName);
-        return true;
+        fun->setPrefixedBoundFunctionName(name);
     }
 
     v.set(name != nullptr ? name : cx->names().empty);
diff --git a/js/src/jsfun.h b/js/src/jsfun.h
index 2bc846b07a..ecdc2f2ebd 100644
--- a/js/src/jsfun.h
+++ b/js/src/jsfun.h
@@ -59,6 +59,9 @@ class JSFunction : public js::NativeObject
         BOUND_FUN        = 0x0008,  /* function was created with Function.prototype.bind. */
         HAS_GUESSED_ATOM = 0x0020,  /* function had no explicit name, but a
                                        name was guessed for it anyway */
+        HAS_BOUND_FUNCTION_NAME_PREFIX = 0x0020, /* bound functions reuse the HAS_GUESSED_ATOM
+                                                    flag to track if atom_ already contains the
+                                                    "bound " function name prefix */
         LAMBDA           = 0x0040,  /* function comes from a FunctionExpression, ArrowFunction, or
                                        Function() call (not a FunctionDeclaration or nonstandard
                                        function-statement) */
@@ -99,8 +102,8 @@ class JSFunction : public js::NativeObject
         INTERPRETED_GENERATOR_OR_ASYNC = INTERPRETED,
         NO_XDR_FLAGS = RESOLVED_LENGTH | RESOLVED_NAME,
 
-        STABLE_ACROSS_CLONES = CONSTRUCTOR | HAS_GUESSED_ATOM | LAMBDA |
-                               SELF_HOSTED | HAS_COMPILE_TIME_NAME | FUNCTION_KIND_MASK
+        STABLE_ACROSS_CLONES = CONSTRUCTOR | LAMBDA | SELF_HOSTED | HAS_COMPILE_TIME_NAME |
+                               FUNCTION_KIND_MASK
     };
 
     static_assert((INTERPRETED | INTERPRETED_LAZY) == js::JS_FUNCTION_INTERPRETED_BITS,
@@ -186,10 +189,20 @@ class JSFunction : public js::NativeObject
     bool isAsmJSNative()            const { return kind() == AsmJS; }
 
     /* Possible attributes of an interpreted function: */
+    bool isBoundFunction()          const { return flags() & BOUND_FUN; }
     bool hasCompileTimeName()       const { return flags() & HAS_COMPILE_TIME_NAME; }
-    bool hasGuessedAtom()           const { return flags() & HAS_GUESSED_ATOM; }
+    bool hasGuessedAtom()           const {
+        static_assert(HAS_GUESSED_ATOM == HAS_BOUND_FUNCTION_NAME_PREFIX,
+                      "HAS_GUESSED_ATOM is unused for bound functions");
+        return (flags() & (HAS_GUESSED_ATOM | BOUND_FUN)) == HAS_GUESSED_ATOM;
+    }
+    bool hasBoundFunctionNamePrefix() const {
+        static_assert(HAS_BOUND_FUNCTION_NAME_PREFIX == HAS_GUESSED_ATOM,
+                      "HAS_BOUND_FUNCTION_NAME_PREFIX is only used for bound functions");
+        MOZ_ASSERT(isBoundFunction());
+        return flags() & HAS_BOUND_FUNCTION_NAME_PREFIX;
+    }
     bool isLambda()                 const { return flags() & LAMBDA; }
-    bool isBoundFunction()          const { return flags() & BOUND_FUN; }
     bool isInterpretedLazy()        const { return flags() & INTERPRETED_LAZY; }
     bool hasScript()                const { return flags() & INTERPRETED; }
 
@@ -347,16 +360,26 @@ class JSFunction : public js::NativeObject
         MOZ_ASSERT(atom);
         MOZ_ASSERT(!hasCompileTimeName());
         MOZ_ASSERT(!hasGuessedAtom());
+        MOZ_ASSERT(!isBoundFunction());
+        MOZ_ASSERT(js::AtomIsMarked(zone(), atom));
         atom_ = atom;
         flags_ |= HAS_GUESSED_ATOM;
     }
     void clearGuessedAtom() {
         MOZ_ASSERT(hasGuessedAtom());
+        MOZ_ASSERT(!isBoundFunction());
         MOZ_ASSERT(atom_);
         atom_ = nullptr;
         flags_ &= ~HAS_GUESSED_ATOM;
     }
 
+    void setPrefixedBoundFunctionName(JSAtom* atom) {
+        MOZ_ASSERT(!hasBoundFunctionNamePrefix());
+        MOZ_ASSERT(atom);
+        flags_ |= HAS_BOUND_FUNCTION_NAME_PREFIX;
+        atom_ = atom;
+    }
+
     /* uint16_t representation bounds number of call object dynamic slots. */
     enum { MAX_ARGS_AND_VARS = 2 * ((1U << 16) - 1) };