Bug 966743 - Inline Array.prototype.push with more than one argument. r=jandem

js/src/jit-test/tests/ion/array-push-multiple-frozen.js

@@ -0,0 +1,87 @@
+// |jit-test| --no-threads
+
+// This test case check's Ion ability to recover from an allocation failure in
+// the inlining of Array.prototype.push, when given multiple arguments. Note,
+// that the following are not equivalent in case of failures:
+//
+//   arr = [];
+//   arr.push(1,2,3); // OOM ---> arr == []
+//
+//   arr = [];
+//   arr.push(1);
+//   arr.push(2); // OOM --> arr == [1]
+//   arr.push(3);
+
+function canIoncompile() {
+  while (true) {
+    var i = inIon();
+    if (i)
+      return i;
+  }
+}
+
+if (!("oomAtAllocation" in this))
+  quit();
+if (canIoncompile() != true)
+  quit();
+if ("gczeal" in this)
+  gczeal(0);
+
+function pushLimits(limit, offset, arr, freeze) {
+  arr = arr || [];
+  arr.push(0,1,2,3,4,5,6,7,8,9);
+  arr.length = offset;
+  var l = arr.length;
+  var was = inIon();
+  oomAtAllocation(limit);
+  try {
+    for (var i = 0; i < 50; i++)
+      arr.push(0,1,2,3,4,5,6,7,8,9);
+    if (freeze)
+      arr.frozen();
+    for (var i = 0; i < 100; i++)
+      arr.push(0,1,2,3,4,5,6,7,8,9);
+  } catch (e) {
+    // Catch OOM.
+  }
+  resetOOMFailure();
+  assertEq(arr.length % 10, l);
+  // Check for a bailout.
+  var is = inIon();
+  return was ? is ? 1 : 2 : 0;
+}
+
+// We need this limit to be high enough to be able to OSR in the for-loop of
+// pushLimits.
+var limit = 1024 * 1024 * 1024;
+while(true) {
+  var res = pushLimits(limit, 0);
+
+  if (res == 0) {
+    limit = 1024 * 1024 * 1024;
+  } else if (res == 1) { // Started and finished in Ion.
+    if (limit == 0) // If we are not in the Jit.
+      break;
+    // We want to converge quickly to a state where the memory is limited
+    // enough to cause failures in array.prototype.push.
+    limit = (limit / 2) | 0;
+  } else if (res == 2) { // Started in Ion, and finished in Baseline.
+    if (limit < 10) {
+      // This is used to offset the OOM location, such that we can test
+      // each steps of the Array.push function, when it is jitted.
+      for (var off = 1; off < 10; off++) {
+        var arr = [];
+        try {
+          pushLimits(limit, off, arr, true);
+        } catch (e) {
+          // Cacth OOM produced while generating the error message.
+        }
+        if (arr.length > 10) assertEq(arr[arr.length - 1], 9);
+        else assertEq(arr[arr.length - 1], arr.length - 1);
+      }
+    }
+    if (limit == 1)
+      break;
+    limit--;
+  }
+}

js/src/jit-test/tests/ion/array-push-multiple.js

@@ -0,0 +1,74 @@
+// |jit-test| --no-threads
+
+// This test case check's Ion ability to recover from an allocation failure in
+// the inlining of Array.prototype.push, when given multiple arguments. Note,
+// that the following are not equivalent in case of failures:
+//
+//   arr = [];
+//   arr.push(1,2,3); // OOM ---> arr == []
+//
+//   arr = [];
+//   arr.push(1);
+//   arr.push(2); // OOM --> arr == [1]
+//   arr.push(3);
+
+function canIoncompile() {
+  while (true) {
+    var i = inIon();
+    if (i)
+      return i;
+  }
+}
+
+if (!("oomAtAllocation" in this))
+  quit();
+if (canIoncompile() != true)
+  quit();
+if ("gczeal" in this)
+  gczeal(0);
+
+function pushLimits(limit, offset) {
+  var arr = [0,1,2,3,4,5,6,7,8,9];
+  arr.length = offset;
+  var l = arr.length;
+  var was = inIon();
+  oomAtAllocation(limit);
+  try {
+    for (var i = 0; i < 100; i++)
+      arr.push(0,1,2,3,4,5,6,7,8,9);
+  } catch (e) {
+    // Catch OOM.
+  }
+  resetOOMFailure();
+  assertEq(arr.length % 10, l);
+  // Check for a bailout.
+  var is = inIon();
+  return was ? is ? 1 : 2 : 0;
+}
+
+// We need this limit to be high enough to be able to OSR in the for-loop of
+// pushLimits.
+var limit = 1024 * 1024 * 1024;
+while(true) {
+  var res = pushLimits(limit, 0);
+
+  if (res == 0) {
+    limit = 1024 * 1024 * 1024;
+  } else if (res == 1) { // Started and finished in Ion.
+    if (limit == 0) // If we are not in the Jit.
+      break;
+    // We want to converge quickly to a state where the memory is limited
+    // enough to cause failures in array.prototype.push.
+    limit = (limit / 2) | 0;
+  } else if (res == 2) { // Started in Ion, and finished in Baseline.
+    if (limit < 10) {
+      // This is used to offset the OOM location, such that we can test
+      // each steps of the Array.push function, when it is jitted.
+      for (var off = 1; off < 10; off++)
+        pushLimits(limit, off);
+    }
+    if (limit == 1)
+      break;
+    limit--;
+  }
+}

js/src/jit/MCallOptimize.cpp

@@ -790,18 +790,21 @@ IonBuilder::inlineArrayJoin(CallInfo& callInfo)
 IonBuilder::InliningResult
 IonBuilder::inlineArrayPush(CallInfo& callInfo)
 {
-    if (callInfo.argc() != 1 || callInfo.constructing()) {
+    const uint32_t inlineArgsLimit = 10;
+    if (callInfo.argc() < 1 || callInfo.argc() > inlineArgsLimit || callInfo.constructing()) {
         trackOptimizationOutcome(TrackedOutcome::CantInlineNativeBadForm);
         return InliningStatus_NotInlined;
     }
 
     MDefinition* obj = convertUnboxedObjects(callInfo.thisArg());
-    MDefinition* value = callInfo.getArg(0);
-    if (PropertyWriteNeedsTypeBarrier(alloc(), constraints(), current,
-                                      &obj, nullptr, &value, /* canModify = */ false))
-    {
-        trackOptimizationOutcome(TrackedOutcome::NeedsTypeBarrier);
-        return InliningStatus_NotInlined;
+    for (uint32_t i = 0; i < callInfo.argc(); i++) {
+        MDefinition* value = callInfo.getArg(i);
+        if (PropertyWriteNeedsTypeBarrier(alloc(), constraints(), current,
+                                          &obj, nullptr, &value, /* canModify = */ false))
+        {
+            trackOptimizationOutcome(TrackedOutcome::NeedsTypeBarrier);
+            return InliningStatus_NotInlined;
+        }
     }
 
     if (getInlineReturnType() != MIRType::Int32)
@@ -845,24 +848,73 @@ IonBuilder::inlineArrayPush(CallInfo& callInfo)
 
     callInfo.setImplicitlyUsedUnchecked();
 
-    if (conversion == TemporaryTypeSet::AlwaysConvertToDoubles ||
-        conversion == TemporaryTypeSet::MaybeConvertToDoubles)
-    {
-        MInstruction* valueDouble = MToDouble::New(alloc(), value);
-        current->add(valueDouble);
-        value = valueDouble;
-    }
+    bool toDouble =
+        conversion == TemporaryTypeSet::AlwaysConvertToDoubles ||
+        conversion == TemporaryTypeSet::MaybeConvertToDoubles;
 
     if (unboxedType == JSVAL_TYPE_MAGIC)
         obj = addMaybeCopyElementsForWrite(obj, /* checkNative = */ false);
 
-    if (needsPostBarrier(value))
-        current->add(MPostWriteBarrier::New(alloc(), obj, value));
+    // If we have more than one argument, we are splitting the function into
+    // multiple inlined calls to Array.push.
+    //
+    // Still, in case of bailouts, we have to keep the atomicity of the call, as
+    // we cannot resume within Array.push function. To do so, we register a
+    // resume point which would be captured by the upcoming MArrayPush
+    // instructions, and this resume point contains an instruction which
+    // truncates the length of the Array, to its original length in case of
+    // bailouts, and resume before the Array.push call.
+    MResumePoint* lastRp = nullptr;
+    MInstruction* truncate = nullptr;
+    if (callInfo.argc() > 1) {
+        MInstruction* elements = MElements::New(alloc(), obj);
+        MInstruction* length = MArrayLength::New(alloc(), elements);
+        truncate = MSetArrayLength::New(alloc(), obj, length);
+        truncate->setRecoveredOnBailout();
 
-    MArrayPush* ins = MArrayPush::New(alloc(), obj, value, unboxedType);
-    current->add(ins);
+        current->add(elements);
+        current->add(length);
+        current->add(truncate);
+
+        // Restore the stack, such that resume points are created with the stack
+        // as it was before the call.
+        callInfo.pushFormals(current);
+    }
+
+    MInstruction* ins = nullptr;
+    for (uint32_t i = 0; i < callInfo.argc(); i++) {
+        MDefinition* value = callInfo.getArg(i);
+        if (toDouble) {
+            MInstruction* valueDouble = MToDouble::New(alloc(), value);
+            current->add(valueDouble);
+            value = valueDouble;
+        }
+
+        if (needsPostBarrier(value))
+            current->add(MPostWriteBarrier::New(alloc(), obj, value));
+
+        ins = MArrayPush::New(alloc(), obj, value, unboxedType);
+        current->add(ins);
+
+        if (callInfo.argc() > 1) {
+            // Restore that call stack and the array length.
+            MOZ_TRY(resumeAt(ins, pc));
+            ins->resumePoint()->addStore(alloc(), truncate, lastRp);
+            lastRp = ins->resumePoint();
+        }
+    }
+
+    if (callInfo.argc() > 1) {
+        // Fix the stack to represent the state after the call execution.
+        callInfo.popFormals(current);
+    }
     current->push(ins);
 
+    if (callInfo.argc() > 1) {
+        ins = MNop::New(alloc());
+        current->add(ins);
+    }
+
     MOZ_TRY(resumeAfter(ins));
     return InliningStatus_Inlined;
 }

js/src/jit/MIR.h

@@ -9226,6 +9226,12 @@ class MSetArrayLength
     AliasSet getAliasSet() const override {
         return AliasSet::Store(AliasSet::ObjectFields);
     }
+
+    // By default no, unless built as a recovered instruction.
+    MOZ_MUST_USE bool writeRecoverData(CompactBufferWriter& writer) const override;
+    bool canRecoverOnBailout() const override {
+        return isRecoveredOnBailout();
+    }
 };
 
 class MGetNextEntryForIterator

js/src/jit/Recover.cpp

@@ -1687,6 +1687,38 @@ RArrayState::recover(JSContext* cx, SnapshotIterator& iter) const
     return true;
 }
 
+bool
+MSetArrayLength::writeRecoverData(CompactBufferWriter& writer) const
+{
+    MOZ_ASSERT(canRecoverOnBailout());
+    // For simplicity, we capture directly the object instead of the elements
+    // pointer.
+    MOZ_ASSERT(elements()->type() != MIRType::Elements);
+    writer.writeUnsigned(uint32_t(RInstruction::Recover_SetArrayLength));
+    return true;
+}
+
+RSetArrayLength::RSetArrayLength(CompactBufferReader& reader)
+{
+}
+
+bool
+RSetArrayLength::recover(JSContext* cx, SnapshotIterator& iter) const
+{
+    RootedValue result(cx);
+    RootedArrayObject obj(cx, &iter.read().toObject().as<ArrayObject>());
+    RootedValue len(cx, iter.read());
+
+    RootedId id(cx, NameToId(cx->names().length));
+    ObjectOpResult error;
+    if (!ArraySetLength(cx, obj, id, JSPROP_PERMANENT, len, error))
+        return false;
+
+    result.setObject(*obj);
+    iter.storeInstructionResult(result);
+    return true;
+}
+
 bool
 MAssertRecoveredOnBailout::writeRecoverData(CompactBufferWriter& writer) const
 {

js/src/jit/Recover.h

@@ -110,6 +110,7 @@ namespace jit {
     _(SimdBox)                                  \
     _(ObjectState)                              \
     _(ArrayState)                               \
+    _(SetArrayLength)                           \
     _(AtomicIsLockFree)                         \
     _(AssertRecoveredOnBailout)
 
@@ -688,6 +689,14 @@ class RArrayState final : public RInstruction
     MOZ_MUST_USE bool recover(JSContext* cx, SnapshotIterator& iter) const override;
 };
 
+class RSetArrayLength final : public RInstruction
+{
+  public:
+    RINSTRUCTION_HEADER_NUM_OP_(SetArrayLength, 2)
+
+    MOZ_MUST_USE bool recover(JSContext* cx, SnapshotIterator& iter) const override;
+};
+
 class RAtomicIsLockFree final : public RInstruction
 {
   public: