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 86061 57c19a4e2d50fbba067ef806a1d11b8b12843781
parent 86060 d0c192e5bd41345e6acdc497e820150ae9aec484
child 86062 ece333215bde81e1ed9f984d64991cb0adfe7c7e
push idunknown
push userunknown
push dateunknown
reviewersdvander
bugs713754
milestone12.0a1
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);