Bug 602131. Make the 'in' operator trace usefully when its right-hand side is a dense array. r=jorendorff
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 23 Nov 2010 17:23:02 -0500
changeset 58285 b87a31d4c0718013d1726888044f37b13b6b3886
parent 58284 96c9432d41cf7349f7a9a5422d14e9910fc6133e
child 58286 d3adb226abe7f44876349d86c6e1a9cd56c3d458
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewersjorendorff
bugs602131
milestone2.0b8pre
Bug 602131. Make the 'in' operator trace usefully when its right-hand side is a dense array. r=jorendorff
js/src/jstracer.cpp
js/src/trace-test/tests/basic/testArrayIn.js
js/src/trace-test/tests/basic/testArrayInWithIndexedProto.js
--- a/js/src/jstracer.cpp
+++ b/js/src/jstracer.cpp
@@ -12901,23 +12901,17 @@ TraceRecorder::setElem(int lval_spindex,
         LIns *elemp_ins = w.name(w.getDslotAddress(obj_ins, idx_ins), "elemp");
 
         // If we are overwriting a hole:
         // - Guard that we don't have any indexed properties along the prototype chain.
         // - Check if the length has changed;  if so, update it to index+1.
         // This happens moderately often, eg. close to 10% of the time in
         // SunSpider, and for some benchmarks it's close to 100%.
         Address dslotAddr = DSlotsAddress(elemp_ins);
-        LIns* isHole_ins = w.name(w.eqi(
-#if JS_BITS_PER_WORD == 32
-                                        w.ldiValueTag(dslotAddr),
-#else
-                                        w.q2i(w.rshuqN(w.ldq(dslotAddr), JSVAL_TAG_SHIFT)),
-#endif
-                                        w.nameImmui(JSVAL_TAG_MAGIC)),
+        LIns* isHole_ins = w.name(is_boxed_magic(dslotAddr, JS_ARRAY_HOLE),
                                   "isHole");
         w.pauseAddingCSEValues();
         if (MaybeBranch mbr1 = w.jf(isHole_ins)) {
             /*
              * It's important that this use branchExit, not mismatchExit, since
              * changes to shapes should just mean we compile a new branch, not
              * throw the whole trace away.
              */
@@ -14776,20 +14770,54 @@ TraceRecorder::record_JSOP_IN()
     JSObject* obj = &rval.toObject();
     LIns* obj_ins = get(&rval);
 
     jsid id;
     LIns* x;
     if (lval.isInt32()) {
         if (!js_Int32ToId(cx, lval.toInt32(), &id))
             RETURN_ERROR_A("OOM converting left operand of JSOP_IN to string");
-        LIns* num_ins;
-        CHECK_STATUS_A(makeNumberInt32(get(&lval), &num_ins));
-        LIns* args[] = { num_ins, obj_ins, cx_ins };
-        x = w.call(&js_HasNamedPropertyInt32_ci, args);
+
+        if (obj->isDenseArray()) {
+            // Fast path for dense arrays
+            VMSideExit* branchExit = snapshot(BRANCH_EXIT);
+            guardDenseArray(obj_ins, branchExit);
+
+            // If our proto has indexed props, all bets are off on our
+            // "false" values and out-of-bounds access.  Just guard on
+            // that.
+            CHECK_STATUS_A(guardPrototypeHasNoIndexedProperties(obj, obj_ins,
+                                                                snapshot(MISMATCH_EXIT)));
+
+            LIns* idx_ins;
+            CHECK_STATUS_A(makeNumberInt32(get(&lval), &idx_ins));
+            idx_ins = w.name(idx_ins, "index");
+            LIns* capacity_ins = w.ldiDenseArrayCapacity(obj_ins);
+            LIns* inRange = w.ltui(idx_ins, capacity_ins);
+
+            if (jsuint(lval.toInt32()) < obj->getDenseArrayCapacity()) {
+                guard(true, inRange, branchExit);
+
+                LIns *elem_ins = w.getDslotAddress(obj_ins, idx_ins);
+                // Need to make sure we don't have a hole
+                LIns *is_hole_ins =
+                    is_boxed_magic(DSlotsAddress(elem_ins), JS_ARRAY_HOLE);
+
+                // Set x to true (index in our array) if is_hole_ins == 0
+                x = w.eqi0(is_hole_ins);
+            } else {
+                guard(false, inRange, branchExit);
+                x = w.nameImmi(0);
+            }
+        } else {
+            LIns* num_ins;
+            CHECK_STATUS_A(makeNumberInt32(get(&lval), &num_ins));
+            LIns* args[] = { num_ins, obj_ins, cx_ins };
+            x = w.call(&js_HasNamedPropertyInt32_ci, args);
+        }
     } else if (lval.isString()) {
         if (!js_ValueToStringId(cx, lval, &id))
             RETURN_ERROR_A("left operand of JSOP_IN didn't convert to a string-id");
         LIns* args[] = { get(&lval), obj_ins, cx_ins };
         x = w.call(&js_HasNamedProperty_ci, args);
     } else {
         RETURN_STOP_A("string or integer expected");
     }
new file mode 100644
--- /dev/null
+++ b/js/src/trace-test/tests/basic/testArrayIn.js
@@ -0,0 +1,54 @@
+// Give us enough room to work with some holes too
+const array_size = RUNLOOP + 20;
+
+function testArrayIn()
+{
+    var arr = new Array(array_size);
+    var i;
+    for (i = 0; i < array_size; ++i) {
+	arr[i] = i;
+    }
+
+    // Sanity check here
+    checkStats({
+	traceCompleted: 1,
+	traceTriggered: 1,
+	sideExitIntoInterpreter: 1
+    });
+    
+    delete arr[RUNLOOP + 5];
+    delete arr[RUNLOOP + 10];
+
+    var ret = [];
+    for (i = 0; i < array_size; ++i) {
+	ret.push(i in arr);
+    }
+
+    checkStats({
+	traceCompleted: 2,
+	traceTriggered: 2,
+	sideExitIntoInterpreter: 2
+    });
+
+    
+    var ret2;
+    for (i = 0; i < RUNLOOP; ++i) {
+	ret2 = array_size in arr;
+    }
+
+    checkStats({
+	traceCompleted: 3,
+	traceTriggered: 3,
+	sideExitIntoInterpreter: 3
+    });
+
+    return [ret, ret2];
+}
+
+var [ret, ret2] = testArrayIn();
+
+assertEq(ret2, false);
+
+for (var i = 0; i < array_size; ++i) {
+    assertEq(ret[i], i != RUNLOOP + 5 && i != RUNLOOP + 10);
+}
new file mode 100644
--- /dev/null
+++ b/js/src/trace-test/tests/basic/testArrayInWithIndexedProto.js
@@ -0,0 +1,31 @@
+function testArrayInWithIndexedProto()
+{
+    Array.prototype[0] = "Got me";
+    var zeroPresent, zeroPresent2;
+    // Need to go to 2*RUNLOOP because in the failure mode this is
+    // testing we have various side-exits in there due to interp and
+    // tracer not agreeing that confuse the issue and cause us to not
+    // hit the bad case within RUNLOOP iterations.
+    for (var j = 0; j < 2*RUNLOOP; ++j) {
+	zeroPresent = 0 in [];
+    }
+
+    var arr = [1, 2];
+    delete arr[0];
+    for (var j = 0; j < 2*RUNLOOP; ++j) {
+	zeroPresent2 = 0 in arr;
+    }
+    return [zeroPresent, zeroPresent2];
+}
+
+var [ret, ret2] = testArrayInWithIndexedProto();
+assertEq(ret, true);
+assertEq(ret2, true);
+
+checkStats({
+    traceCompleted: 0,
+    traceTriggered: 0,
+    sideExitIntoInterpreter: 0,
+    recorderStarted: 2,
+    recorderAborted: 2
+});