Bug 1256298 - Make DoCallFallback consume a bit less stack space. r=Waldo
authorJan de Mooij <jdemooij@mozilla.com>
Sat, 26 Mar 2016 19:52:30 +0100
changeset 290574 ca768ab10b5bf6ac295cb2470bb50fea14cb5324
parent 290573 2a8ca5caae8e3a300701b0a85e5051ead55e0648
child 290575 d26b3c84b32337f675506dbf2fb55261acfc6c57
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs1256298
milestone48.0a1
Bug 1256298 - Make DoCallFallback consume a bit less stack space. r=Waldo
js/src/jit/BaselineIC.cpp
js/src/vm/Interpreter.cpp
js/src/vm/Interpreter.h
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -5990,23 +5990,22 @@ CopyArray(JSContext* cx, HandleObject ob
     CopyAnyBoxedOrUnboxedDenseElements(cx, nobj, obj, 0, 0, length);
 
     result.setObject(*nobj);
     return true;
 }
 
 static bool
 TryAttachStringSplit(JSContext* cx, ICCall_Fallback* stub, HandleScript script,
-                     uint32_t argc, Value* vp, jsbytecode* pc, HandleValue res,
-                     bool* attached)
+                     uint32_t argc, HandleValue callee, Value* vp, jsbytecode* pc,
+                     HandleValue res, bool* attached)
 {
     if (stub->numOptimizedStubs() != 0)
         return true;
 
-    RootedValue callee(cx, vp[0]);
     RootedValue thisv(cx, vp[1]);
     Value* args = vp + 2;
 
     // String.prototype.split will not yield a constructable.
     if (JSOp(*pc) == JSOP_NEW)
         return true;
 
     if (!IsOptimizableCallStringSplit(callee, thisv, argc, args))
@@ -6063,87 +6062,64 @@ DoCallFallback(JSContext* cx, BaselineFr
     jsbytecode* pc = stub->icEntry()->pc(script);
     JSOp op = JSOp(*pc);
     FallbackICSpew(cx, stub, "Call(%s)", CodeName[op]);
 
     MOZ_ASSERT(argc == GET_ARGC(pc));
     bool constructing = (op == JSOP_NEW);
 
     // Ensure vp array is rooted - we may GC in here.
-    AutoArrayRooter vpRoot(cx, argc + 2 + constructing, vp);
-
+    size_t numValues = argc + 2 + constructing;
+    AutoArrayRooter vpRoot(cx, numValues, vp);
+
+    CallArgs callArgs = CallArgsFromSp(argc + constructing, vp + numValues, constructing);
     RootedValue callee(cx, vp[0]);
-    RootedValue thisv(cx, vp[1]);
-
-    Value* args = vp + 2;
 
     // Handle funapply with JSOP_ARGUMENTS
-    if (op == JSOP_FUNAPPLY && argc == 2 && args[1].isMagic(JS_OPTIMIZED_ARGUMENTS)) {
-        CallArgs callArgs = CallArgsFromVp(argc, vp);
+    if (op == JSOP_FUNAPPLY && argc == 2 && callArgs[1].isMagic(JS_OPTIMIZED_ARGUMENTS)) {
         if (!GuardFunApplyArgumentsOptimization(cx, frame, callArgs))
             return false;
     }
 
     bool createSingleton = ObjectGroup::useSingletonForNewObject(cx, script, pc);
 
     // Try attaching a call stub.
     bool handled = false;
     if (!TryAttachCallStub(cx, stub, script, pc, op, argc, vp, constructing, false,
                            createSingleton, &handled))
     {
         return false;
     }
 
     if (op == JSOP_NEW) {
-        // Callees from the stack could have any old non-constructor callee.
-        if (!IsConstructor(callee)) {
-            ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, callee, nullptr);
-            return false;
-        }
-
-        ConstructArgs cargs(cx);
-        if (!cargs.init(argc))
+        if (!ConstructFromStack(cx, callArgs))
             return false;
-
-        for (uint32_t i = 0; i < argc; i++)
-            cargs[i].set(args[i]);
-
-        RootedValue newTarget(cx, args[argc]);
-        MOZ_ASSERT(IsConstructor(newTarget),
-                   "either callee == newTarget, or the initial |new| checked "
-                   "that IsConstructor(newTarget)");
-
-        RootedObject obj(cx);
-        if (!Construct(cx, callee, cargs, newTarget, &obj))
-            return false;
-
-        res.setObject(*obj);
-
     } else if ((op == JSOP_EVAL || op == JSOP_STRICTEVAL) &&
                frame->scopeChain()->global().valueIsEval(callee))
     {
-        if (!DirectEval(cx, CallArgsFromVp(argc, vp)))
+        if (!DirectEval(cx, callArgs))
             return false;
-        res.set(vp[0]);
     } else {
         MOZ_ASSERT(op == JSOP_CALL ||
                    op == JSOP_CALLITER ||
                    op == JSOP_FUNCALL ||
                    op == JSOP_FUNAPPLY ||
                    op == JSOP_EVAL ||
                    op == JSOP_STRICTEVAL);
         if (op == JSOP_CALLITER && callee.isPrimitive()) {
             MOZ_ASSERT(argc == 0, "thisv must be on top of the stack");
-            ReportValueError(cx, JSMSG_NOT_ITERABLE, -1, thisv, nullptr);
+            ReportValueError(cx, JSMSG_NOT_ITERABLE, -1, callArgs.thisv(), nullptr);
             return false;
         }
-        if (!Invoke(cx, thisv, callee, argc, args, res))
+
+        if (!Invoke(cx, callArgs))
             return false;
     }
 
+    res.set(callArgs.rval());
     TypeScript::Monitor(cx, script, pc, res);
 
     // Check if debug mode toggling made the stub invalid.
     if (stub.invalid())
         return true;
 
     // Attach a new TypeMonitor stub for this value.
     ICTypeMonitor_Fallback* typeMonFbStub = stub->fallbackMonitorStub();
@@ -6152,18 +6128,19 @@ DoCallFallback(JSContext* cx, BaselineFr
         return false;
     }
 
     // Add a type monitor stub for the resulting value.
     if (!stub->addMonitorStubForValue(cx, &info, res))
         return false;
 
     // If 'callee' is a potential Call_StringSplit, try to attach an
-    // optimized StringSplit stub.
-    if (!TryAttachStringSplit(cx, stub, script, argc, vp, pc, res, &handled))
+    // optimized StringSplit stub. Note that vp[0] now holds the return value
+    // instead of the callee, so we pass the callee as well.
+    if (!TryAttachStringSplit(cx, stub, script, argc, callee, vp, pc, res, &handled))
         return false;
 
     if (!handled)
         stub->noteUnoptimizableCall();
     return true;
 }
 
 static bool
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -577,18 +577,18 @@ StackCheckIsConstructorCalleeNewTarget(J
 
     // The new.target has already been vetted by previous calls, or is the callee.
     // We can just assert that it's a constructor.
     MOZ_ASSERT(IsConstructor(newTarget));
 
     return true;
 }
 
-static bool
-ConstructFromStack(JSContext* cx, const CallArgs& args)
+bool
+js::ConstructFromStack(JSContext* cx, const CallArgs& args)
 {
     if (!StackCheckIsConstructorCalleeNewTarget(cx, args.calleev(), args.newTarget()))
         return false;
 
     args.setThis(MagicValue(JS_IS_CONSTRUCTING));
     return InternalConstruct(cx, static_cast<const AnyConstructArgs&>(args));
 }
 
--- a/js/src/vm/Interpreter.h
+++ b/js/src/vm/Interpreter.h
@@ -84,16 +84,28 @@ InvokeSetter(JSContext* cx, const Value&
 // callee as |newTarget| when a different value should have been passed.
 //
 // NOTE: As with the ES6 spec operation, it's the caller's responsibility to
 //       ensure |fval| and |newTarget| are both |IsConstructor|.
 extern bool
 Construct(JSContext* cx, HandleValue fval, const AnyConstructArgs& args, HandleValue newTarget,
           MutableHandleObject objp);
 
+// Check that in the given |args|, which must be |args.isConstructing()|, that
+// |IsConstructor(args.callee())|. If this is not the case, throw a TypeError.
+// Otherwise, the user must ensure that, additionally, |IsConstructor(args.newTarget())|.
+// (If |args| comes directly from the interpreter stack, as set up by JSOP_NEW,
+// this comes for free.) Then perform a Construct() operation using |args|.
+//
+// This internal operation is intended only for use with arguments known to be
+// on the JS stack, or at least in carefully-rooted memory. The vast majority of
+// potential users should instead use ConstructArgs in concert with Construct().
+extern bool
+ConstructFromStack(JSContext* cx, const CallArgs& args);
+
 // Call Construct(fval, args, newTarget), but use the given |thisv| as |this|
 // during construction of |fval|.
 //
 // This method exists only for very rare cases where a |this| was created
 // caller-side for construction of |fval|: basically only for JITs using
 // |CreateThis|.  If that's not you, use Construct()!
 extern bool
 InternalConstructWithProvidedThis(JSContext* cx, HandleValue fval, HandleValue thisv,