Store JSStrings instead of jsids in native key iterators, bug 713754. r=dvander
authorBrian Hackett <bhackett1024@gmail.com>
Wed, 18 Jan 2012 16:56:22 -0800
changeset 84836 57c19a4e2d50fbba067ef806a1d11b8b12843781
parent 84835 d0c192e5bd41345e6acdc497e820150ae9aec484
child 84837 ece333215bde81e1ed9f984d64991cb0adfe7c7e
push id21879
push usermak77@bonardo.net
push dateThu, 19 Jan 2012 10:34:58 +0000
treeherdermozilla-central@f76b576a9e28 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs713754
milestone12.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
Store JSStrings instead of jsids in native key iterators, bug 713754. r=dvander
js/src/jsatominlines.h
js/src/jsinterp.cpp
js/src/jsiter.cpp
js/src/jsiter.h
js/src/jswrapper.cpp
js/src/methodjit/Compiler.cpp
--- a/js/src/jsatominlines.h
+++ b/js/src/jsatominlines.h
@@ -172,21 +172,21 @@ IndexToId(JSContext *cx, uint32_t index,
         *idp = INT_TO_JSID(index);
         return true;
     }
 
     extern bool IndexToIdSlow(JSContext *cx, uint32_t index, jsid *idp);
     return IndexToIdSlow(cx, index, idp);
 }
 
-static JS_ALWAYS_INLINE JSString *
+static JS_ALWAYS_INLINE JSFlatString *
 IdToString(JSContext *cx, jsid id)
 {
     if (JSID_IS_STRING(id))
-        return JSID_TO_STRING(id);
+        return JSID_TO_ATOM(id);
     if (JS_LIKELY(JSID_IS_INT(id)))
-        return js_IntToString(cx, JSID_TO_INT(id));
-    return js::ToStringSlow(cx, IdToValue(id));
+        return js_IntToString(cx, JSID_TO_INT(id))->ensureFlat(cx);
+    return ToStringSlow(cx, IdToValue(id))->ensureFlat(cx);
 }
 
 } // namespace js
 
 #endif /* jsatominlines_h___ */
--- a/js/src/jsinterp.cpp
+++ b/js/src/jsinterp.cpp
@@ -1345,23 +1345,19 @@ IteratorMore(JSContext *cx, JSObject *it
 
 static inline bool
 IteratorNext(JSContext *cx, JSObject *iterobj, Value *rval)
 {
     if (iterobj->isIterator()) {
         NativeIterator *ni = iterobj->getNativeIterator();
         if (ni->isKeyIter()) {
             JS_ASSERT(ni->props_cursor < ni->props_end);
-            jsid id = *ni->current();
-            if (JSID_IS_ATOM(id)) {
-                rval->setString(JSID_TO_STRING(id));
-                ni->incCursor();
-                return true;
-            }
-            /* Take the slow path if we have to stringify a numeric property name. */
+            rval->setString(*ni->current());
+            ni->incCursor();
+            return true;
         }
     }
     return js_IteratorNext(cx, iterobj, rval);
 }
 
 /*
  * For bytecodes which push values and then fall through, make sure the
  * types of the pushed values are consistent with type inference information.
--- a/js/src/jsiter.cpp
+++ b/js/src/jsiter.cpp
@@ -116,17 +116,18 @@ Class js::IteratorClass = {
     }
 };
 
 static const gc::AllocKind ITERATOR_FINALIZE_KIND = gc::FINALIZE_OBJECT2;
 
 void
 NativeIterator::mark(JSTracer *trc)
 {
-    MarkIdRange(trc, begin(), end(), "props");
+    for (HeapPtr<JSFlatString> *str = begin(); str < end(); str++)
+        MarkString(trc, *str, "prop");
     if (obj)
         MarkObject(trc, obj, "obj");
 }
 
 static void
 iterator_finalize(JSContext *cx, JSObject *obj)
 {
     JS_ASSERT(obj->isIterator());
@@ -502,24 +503,30 @@ NewIteratorObject(JSContext *cx, uintN f
     return NewBuiltinClassInstance(cx, &IteratorClass);
 }
 
 NativeIterator *
 NativeIterator::allocateIterator(JSContext *cx, uint32_t slength, const AutoIdVector &props)
 {
     size_t plength = props.length();
     NativeIterator *ni = (NativeIterator *)
-        cx->malloc_(sizeof(NativeIterator) + plength * sizeof(jsid) + slength * sizeof(Shape *));
+        cx->malloc_(sizeof(NativeIterator)
+                    + plength * sizeof(JSString *)
+                    + slength * sizeof(Shape *));
     if (!ni)
         return NULL;
-    ni->props_array = ni->props_cursor = (HeapId *) (ni + 1);
+    ni->props_array = ni->props_cursor = (HeapPtr<JSFlatString> *) (ni + 1);
     ni->props_end = ni->props_array + plength;
     if (plength) {
-        for (size_t i = 0; i < plength; i++)
-            ni->props_array[i].init(props[i]);
+        for (size_t i = 0; i < plength; i++) {
+            JSFlatString *str = IdToString(cx, props[i]);
+            if (!str)
+                return NULL;
+            ni->props_array[i].init(str);
+        }
     }
     return ni;
 }
 
 inline void
 NativeIterator::init(JSObject *obj, uintN flags, uint32_t slength, uint32_t key)
 {
     this->obj.init(obj);
@@ -911,46 +918,50 @@ js_CloseIterator(JSContext *cx, JSObject
  * We do not suppress enumeration of a property deleted along an object's
  * prototype chain. Only direct deletions on the object are handled.
  *
  * This function can suppress multiple properties at once. The |predicate|
  * argument is an object which can be called on an id and returns true or
  * false. It also must have a method |matchesAtMostOne| which allows us to
  * stop searching after the first deletion if true.
  */
-template<typename IdPredicate>
+template<typename StringPredicate>
 static bool
-SuppressDeletedPropertyHelper(JSContext *cx, JSObject *obj, IdPredicate predicate)
+SuppressDeletedPropertyHelper(JSContext *cx, JSObject *obj, StringPredicate predicate)
 {
     JSObject *iterobj = cx->enumerators;
     while (iterobj) {
       again:
         NativeIterator *ni = iterobj->getNativeIterator();
         /* This only works for identified surpressed keys, not values. */
         if (ni->isKeyIter() && ni->obj == obj && ni->props_cursor < ni->props_end) {
             /* Check whether id is still to come. */
-            HeapId *props_cursor = ni->current();
-            HeapId *props_end = ni->end();
-            for (HeapId *idp = props_cursor; idp < props_end; ++idp) {
+            HeapPtr<JSFlatString> *props_cursor = ni->current();
+            HeapPtr<JSFlatString> *props_end = ni->end();
+            for (HeapPtr<JSFlatString> *idp = props_cursor; idp < props_end; ++idp) {
                 if (predicate(*idp)) {
                     /*
                      * Check whether another property along the prototype chain
                      * became visible as a result of this deletion.
                      */
                     if (obj->getProto()) {
                         JSObject *proto = obj->getProto();
                         JSObject *obj2;
                         JSProperty *prop;
-                        if (!proto->lookupGeneric(cx, *idp, &obj2, &prop))
+                        jsid id;
+                        if (!ValueToId(cx, StringValue(*idp), &id))
+                            return false;
+                        id = js_CheckForStringIndex(id);
+                        if (!proto->lookupGeneric(cx, id, &obj2, &prop))
                             return false;
                         if (prop) {
                             uintN attrs;
                             if (obj2->isNative())
                                 attrs = ((Shape *) prop)->attributes();
-                            else if (!obj2->getGenericAttributes(cx, *idp, &attrs))
+                            else if (!obj2->getGenericAttributes(cx, id, &attrs))
                                 return false;
 
                             if (attrs & JSPROP_ENUMERATE)
                                 continue;
                         }
                     }
 
                     /*
@@ -963,17 +974,17 @@ SuppressDeletedPropertyHelper(JSContext 
                     /*
                      * No property along the prototype chain stepped in to take the
                      * property's place, so go ahead and delete id from the list.
                      * If it is the next property to be enumerated, just skip it.
                      */
                     if (idp == props_cursor) {
                         ni->incCursor();
                     } else {
-                        for (HeapId *p = idp; p + 1 != props_end; p++)
+                        for (HeapPtr<JSFlatString> *p = idp; p + 1 != props_end; p++)
                             *p = *(p + 1);
                         ni->props_end = ni->end() - 1;
                     }
 
                     /* Don't reuse modified native iterators. */
                     ni->flags |= JSITER_UNREUSABLE;
 
                     if (predicate.matchesAtMostOne())
@@ -981,61 +992,52 @@ SuppressDeletedPropertyHelper(JSContext 
                 }
             }
         }
         iterobj = ni->next;
     }
     return true;
 }
 
-class SingleIdPredicate {
-    jsid id;
+class SingleStringPredicate {
+    JSFlatString *str;
 public:
-    SingleIdPredicate(jsid id) : id(id) {}
+    SingleStringPredicate(JSFlatString *str) : str(str) {}
 
-    bool operator()(jsid id) { return id == this->id; }
+    bool operator()(JSFlatString *str) { return EqualStrings(str, this->str); }
     bool matchesAtMostOne() { return true; }
 };
 
 bool
 js_SuppressDeletedProperty(JSContext *cx, JSObject *obj, jsid id)
 {
-    id = js_CheckForStringIndex(id);
-    return SuppressDeletedPropertyHelper(cx, obj, SingleIdPredicate(id));
+    JSFlatString *str = IdToString(cx, id);
+    if (!str)
+        return false;
+    return SuppressDeletedPropertyHelper(cx, obj, SingleStringPredicate(str));
 }
 
 bool
 js_SuppressDeletedElement(JSContext *cx, JSObject *obj, uint32_t index)
 {
     jsid id;
     if (!IndexToId(cx, index, &id))
         return false;
-    JS_ASSERT(id == js_CheckForStringIndex(id));
-    return SuppressDeletedPropertyHelper(cx, obj, SingleIdPredicate(id));
+    return js_SuppressDeletedProperty(cx, obj, id);
 }
 
 class IndexRangePredicate {
     uint32_t begin, end;
 
   public:
     IndexRangePredicate(uint32_t begin, uint32_t end) : begin(begin), end(end) {}
 
-    bool operator()(jsid id) {
-        if (JSID_IS_INT(id)) {
-            jsint i = JSID_TO_INT(id);
-            return i > 0 && begin <= uint32_t(i) && uint32_t(i) < end;
-        }
-
-        if (JS_LIKELY(JSID_IS_ATOM(id))) {
-            JSAtom *atom = JSID_TO_ATOM(id);
-            uint32_t index;
-            return atom->isIndex(&index) && begin <= index && index < end;
-        }
-
-        return false;
+    bool operator()(JSFlatString *str) {
+        uint32_t index;
+        return str->isIndex(&index) && begin <= index && index < end;
     }
 
     bool matchesAtMostOne() { return false; }
 };
 
 bool
 js_SuppressDeletedElements(JSContext *cx, JSObject *obj, uint32_t begin, uint32_t end)
 {
@@ -1078,17 +1080,20 @@ js_IteratorMore(JSContext *cx, JSObject 
 
             cx->clearPendingException();
             cx->iterValue.setMagic(JS_NO_ITER_VALUE);
             rval->setBoolean(false);
             return true;
         }
     } else {
         JS_ASSERT(!ni->isKeyIter());
-        jsid id = *ni->current();
+        jsid id;
+        if (!ValueToId(cx, StringValue(*ni->current()), &id))
+            return false;
+        id = js_CheckForStringIndex(id);
         ni->incCursor();
         if (!ni->obj->getGeneric(cx, id, rval))
             return false;
         if ((ni->flags & JSITER_KEYVALUE) && !NewKeyValuePair(cx, id, *rval, rval))
             return false;
     }
 
     /* Cache the value returned by iterobj.next() so js_IteratorNext() can find it. */
@@ -1105,17 +1110,17 @@ js_IteratorNext(JSContext *cx, JSObject 
     if (iterobj->isIterator()) {
         /*
          * Implement next directly as all the methods of the native iterator are
          * read-only and permanent.
          */
         NativeIterator *ni = iterobj->getNativeIterator();
         if (ni->isKeyIter()) {
             JS_ASSERT(ni->props_cursor < ni->props_end);
-            *rval = IdToValue(*ni->current());
+            *rval = StringValue(*ni->current());
             ni->incCursor();
 
             if (rval->isString())
                 return true;
 
             JSString *str;
             jsint i;
             if (rval->isInt32() && StaticStrings::hasInt(i = rval->toInt32())) {
--- a/js/src/jsiter.h
+++ b/js/src/jsiter.h
@@ -57,40 +57,40 @@
  */
 #define JSITER_ACTIVE       0x1000
 #define JSITER_UNREUSABLE   0x2000
 
 namespace js {
 
 struct NativeIterator {
     HeapPtrObject obj;
-    HeapId    *props_array;
-    HeapId    *props_cursor;
-    HeapId    *props_end;
+    HeapPtr<JSFlatString> *props_array;
+    HeapPtr<JSFlatString> *props_cursor;
+    HeapPtr<JSFlatString> *props_end;
     const Shape **shapes_array;
     uint32_t  shapes_length;
     uint32_t  shapes_key;
     uint32_t  flags;
     JSObject  *next;  /* Forms cx->enumerators list, garbage otherwise. */
 
     bool isKeyIter() const { return (flags & JSITER_FOREACH) == 0; }
 
-    inline HeapId *begin() const {
+    inline HeapPtr<JSFlatString> *begin() const {
         return props_array;
     }
 
-    inline HeapId *end() const {
+    inline HeapPtr<JSFlatString> *end() const {
         return props_end;
     }
 
     size_t numKeys() const {
         return end() - begin();
     }
 
-    HeapId *current() const {
+    HeapPtr<JSFlatString> *current() const {
         JS_ASSERT(props_cursor < props_end);
         return props_cursor;
     }
 
     void incCursor() {
         props_cursor = props_cursor + 1;
     }
 
--- a/js/src/jswrapper.cpp
+++ b/js/src/jswrapper.cpp
@@ -668,17 +668,21 @@ Reify(JSContext *cx, JSCompartment *orig
      */
     size_t length = ni->numKeys();
     bool isKeyIter = ni->isKeyIter();
     AutoIdVector keys(cx);
     if (length > 0) {
         if (!keys.resize(length))
             return false;
         for (size_t i = 0; i < length; ++i) {
-            keys[i] = ni->begin()[i];
+            jsid id;
+            if (!ValueToId(cx, StringValue(ni->begin()[i]), &id))
+                return false;
+            id = js_CheckForStringIndex(id);
+            keys[i] = id;
             if (!origin->wrapId(cx, &keys[i]))
                 return false;
         }
     }
 
     close.clear();
     if (!js_CloseIterator(cx, iterObj))
         return false;
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -6223,25 +6223,21 @@ mjit::Compiler::iterNext(ptrdiff_t offse
     notFast = masm.branchTest32(Assembler::NonZero, T3, Imm32(JSITER_FOREACH));
     stubcc.linkExit(notFast, Uses(1));
 
     RegisterID T2 = frame.allocReg();
 
     /* Get cursor. */
     masm.loadPtr(Address(T1, offsetof(NativeIterator, props_cursor)), T2);
 
-    /* Test if the jsid is a string. */
+    /* Get the next string in the iterator. */
     masm.loadPtr(T2, T3);
-    masm.move(T3, T4);
-    masm.andPtr(Imm32(JSID_TYPE_MASK), T4);
-    notFast = masm.branchTestPtr(Assembler::NonZero, T4, T4);
-    stubcc.linkExit(notFast, Uses(1));
 
     /* It's safe to increase the cursor now. */
-    masm.addPtr(Imm32(sizeof(jsid)), T2, T4);
+    masm.addPtr(Imm32(sizeof(JSString*)), T2, T4);
     masm.storePtr(T4, Address(T1, offsetof(NativeIterator, props_cursor)));
 
     frame.freeReg(T4);
     frame.freeReg(T1);
     frame.freeReg(T2);
 
     stubcc.leave();
     stubcc.masm.move(Imm32(offset), Registers::ArgReg1);