We can avoid even more string atomization calls when binding a bound function (bug 1371593). The attached patch improves this µ-benchmark from 255ms to 115ms. --- function f() { function fn(){} var t = dateNow(); for (var i = 0; i < 1000000; ++i) { fn.bind().bind(); } return dateNow() - t; } f() ---

bug 1371593 only improved the case when we repeatedly bind the same bound function. This patch improves the idea from bug 1371593 to store unprefixed (without the "bound " prefix) names for bound functions, so we can also improve the case when we call Function.prototype.bind() for the first time on a bound function. Function overview: JSFunction::getBoundFunctionName() Returns the full name of a bound function, that means including the "bound " prefix. If the HAS_BOUND_FUNCTION_NAME_PREFIX function flag is not set, we compute the full name by counting the number of target bound functions and then appending "bound " that many times. JSFunction::getUnresolvedName() JSFunction::infallibleGetUnresolvedName() This function was modified to no longer atomize the function name when called from fun_resolve(). And parts of it were split into JSFunction::infallibleGetUnresolvedName(). JSFunction::finishBoundFunctionInit() - If the target function doesn't have a resolved name and if it is a bound function with HAS_BOUND_FUNCTION_NAME_PREFIX set, directly store the name atom. If HAS_BOUND_FUNCTION_NAME_PREFIX is set, we need to eagerly compute the full prefixed name because we can't compute it lazily in JSFunction::getBoundFunctionName(). - If the target function has a resolved name (or is not a JSFunction) and it's not a bound function, we can also just save the name atom. If it is a bound function, we need to compute the full prefixed name. We need to handle the bound function cases differently because of things like this: --- // f.atom = "f" function f() {} // bf1.atom = "f" var bf1 = f.bind(); Object.defineProperty(bf1, "name", {value: "g"}); // But bf2.atom = "bound g", because we need to store bf1's name at the time bind() was called, which means we can't restore the name when we're calling JSFunction::getBoundFunctionName(). var bf2 = bf1.bind(); assertEq(bf2.name, "bound g"); --- Which also implies that HAS_BOUND_FUNCTION_NAME_PREFIX is a transitive property, so if the target of a bound function has HAS_BOUND_FUNCTION_NAME_PREFIX set, the bound function also needs to set HAS_BOUND_FUNCTION_NAME_PREFIX.

Comment on attachment 8960696 [details] [diff] [review] bug1447362.patch Review of attachment 8960696 [details] [diff] [review]: ----------------------------------------------------------------- Great improvement! ::: js/src/vm/JSFunction.cpp @@ +1392,5 @@ > + if (name->hasTwoByteChars() && !sb.ensureTwoByteChars()) > + return nullptr; > + > + CheckedInt<size_t> len(boundTargets); > + len *= strlen(boundWithSpaceChars); GCC/Clang should inline the strlen, not sure about MSVC. To avoid relying on this optimization, it might be nicer to do this: static constexpr char boundWithSpaceChars[] = "bound "; static constexpr size_t boundWithSpaceCharsLength = mozilla::ArrayLength(boundWithSpaceChars) - 1; // No trailing '\0'. This ArrayLength is a constexpr: https://searchfox.org/mozilla-central/rev/b29daa46443b30612415c35be0a3c9c13b9dc5f6/mfbt/ArrayUtils.h#54-59 And then use boundWithSpaceCharsLength instead of strlen (twice).

Updated per review comments, carrying r+.

(In reply to Jan de Mooij [:jandem] from comment #2) > static constexpr size_t boundWithSpaceCharsLength = > mozilla::ArrayLength(boundWithSpaceChars) - 1; // No trailing '\0'. It's a bit sad that std::char_traits::length() is only constexpr starting with c++17, because then we wouldn't need to handle the trailing '\0' case...

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fda78b7e464578358e20764819cee47cb68ccdd7

Pushed by apavel@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f56e2332d673 Avoid Atomize calls when binding a bound function. r=jandem