Bug 966743 - Inline Array.prototype.push with more than one argument. r=jandem
authorNicolas B. Pierron <nicolas.b.pierron@mozilla.com>
Thu, 07 Sep 2017 13:01:13 +0000
changeset 379553 8b1881ead0b6
parent 379552 5eb5af7c30a9
child 379554 d6e1c5f5a5c3
push id32456
push userarchaeopteryx@coole-files.de
push dateThu, 07 Sep 2017 22:00:40 +0000
treeherdermozilla-central@b4c1ad9565ee [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs966743
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 966743 - Inline Array.prototype.push with more than one argument. r=jandem
js/src/jit-test/tests/ion/array-push-multiple-frozen.js
js/src/jit-test/tests/ion/array-push-multiple.js
js/src/jit/MCallOptimize.cpp
js/src/jit/MIR.h
js/src/jit/Recover.cpp
js/src/jit/Recover.h
new file mode 100644
--- /dev/null
+++ b/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--;
+  }
+}
new file mode 100644
--- /dev/null
+++ b/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--;
+  }
+}
--- a/js/src/jit/MCallOptimize.cpp
+++ b/js/src/jit/MCallOptimize.cpp
@@ -785,28 +785,31 @@ IonBuilder::inlineArrayJoin(CallInfo& ca
 
     MOZ_TRY(resumeAfter(ins));
     return InliningStatus_Inlined;
 }
 
 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)
         return InliningStatus_NotInlined;
     if (obj->type() != MIRType::Object)
         return InliningStatus_NotInlined;
 
     TemporaryTypeSet* thisTypes = obj->resultTypeSet();
@@ -840,34 +843,83 @@ IonBuilder::inlineArrayPush(CallInfo& ca
     if (clasp == &UnboxedArrayObject::class_) {
         unboxedType = UnboxedArrayElementType(constraints(), obj, nullptr);
         if (unboxedType == JSVAL_TYPE_MAGIC)
             return InliningStatus_NotInlined;
     }
 
     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));
-
-    MArrayPush* ins = MArrayPush::New(alloc(), obj, value, unboxedType);
-    current->add(ins);
+    // 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();
+
+        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;
 }
 
 IonBuilder::InliningResult
 IonBuilder::inlineArraySlice(CallInfo& callInfo)
 {
     if (callInfo.constructing()) {
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -9221,16 +9221,22 @@ class MSetArrayLength
   public:
     INSTRUCTION_HEADER(SetArrayLength)
     TRIVIAL_NEW_WRAPPERS
     NAMED_OPERANDS((0, elements), (1, index))
 
     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
   : public MBinaryInstruction,
     public MixPolicy<ObjectPolicy<0>, ObjectPolicy<1> >::Data
 {
   public:
     enum Mode {
--- a/js/src/jit/Recover.cpp
+++ b/js/src/jit/Recover.cpp
@@ -1683,16 +1683,48 @@ RArrayState::recover(JSContext* cx, Snap
     }
 
     result.setObject(*object);
     iter.storeInstructionResult(result);
     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
 {
     MOZ_ASSERT(canRecoverOnBailout());
     MOZ_RELEASE_ASSERT(input()->isRecoveredOnBailout() == mustBeRecovered_,
         "assertRecoveredOnBailout failed during compilation");
     writer.writeUnsigned(uint32_t(RInstruction::Recover_AssertRecoveredOnBailout));
     return true;
 }
--- a/js/src/jit/Recover.h
+++ b/js/src/jit/Recover.h
@@ -105,16 +105,17 @@ namespace jit {
     _(NewIterator)                              \
     _(NewDerivedTypedObject)                    \
     _(CreateThisWithTemplate)                   \
     _(Lambda)                                   \
     _(LambdaArrow)                              \
     _(SimdBox)                                  \
     _(ObjectState)                              \
     _(ArrayState)                               \
+    _(SetArrayLength)                           \
     _(AtomicIsLockFree)                         \
     _(AssertRecoveredOnBailout)
 
 class RResumePoint;
 class SnapshotIterator;
 
 class MOZ_NON_PARAM RInstruction
 {
@@ -683,16 +684,24 @@ class RArrayState final : public RInstru
         // +1 for the array.
         // +1 for the initalized length.
         return numElements() + 2;
     }
 
     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:
     RINSTRUCTION_HEADER_NUM_OP_(AtomicIsLockFree, 1)
 
     MOZ_MUST_USE bool recover(JSContext* cx, SnapshotIterator& iter) const override;
 };