Bug 1191098 - Replace AutoHashableValueRooter with Rooted<HashableValue>; r=jonco
authorTerrence Cole <terrence@mozilla.com>
Tue, 04 Aug 2015 09:23:27 -0700
changeset 288096 21c28216e5e7255fa072ceefe2f31b1930758f34
parent 288095 fe4520e7fc27fea9b7555efb5befcfa23ea82df0
child 288097 6249e4edb023c89661910a5838f7442b72411a10
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1191098
milestone42.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 1191098 - Replace AutoHashableValueRooter with Rooted<HashableValue>; r=jonco
js/src/builtin/MapObject.cpp
js/src/builtin/MapObject.h
js/src/gc/RootMarking.cpp
js/src/jspubtd.h
--- a/js/src/builtin/MapObject.cpp
+++ b/js/src/builtin/MapObject.cpp
@@ -397,26 +397,26 @@ MapObject::getKeysAndValuesInterleaved(J
 
 bool
 MapObject::set(JSContext* cx, HandleObject obj, HandleValue k, HandleValue v)
 {
     ValueMap* map = obj->as<MapObject>().getData();
     if (!map)
         return false;
 
-    AutoHashableValueRooter key(cx);
+    Rooted<HashableValue> key(cx);
     if (!key.setValue(cx, k))
         return false;
 
     RelocatableValue rval(v);
     if (!map->put(key, rval)) {
         ReportOutOfMemory(cx);
         return false;
     }
-    WriteBarrierPost(cx->runtime(), map, key.get());
+    WriteBarrierPost(cx->runtime(), map, key.value());
     return true;
 }
 
 MapObject*
 MapObject::create(JSContext* cx)
 {
     Rooted<MapObject*> obj(cx, NewBuiltinClassInstance<MapObject>(cx));
     if (!obj)
@@ -465,17 +465,17 @@ MapObject::construct(JSContext* cx, unsi
         FastInvokeGuard fig(cx, adderVal);
         InvokeArgs& args2 = fig.args();
 
         ForOfIterator iter(cx);
         if (!iter.init(args[0]))
             return false;
         RootedValue pairVal(cx);
         RootedObject pairObj(cx);
-        AutoHashableValueRooter hkey(cx);
+        Rooted<HashableValue> hkey(cx);
         ValueMap* map = obj->getData();
         while (true) {
             bool done;
             if (!iter.next(&pairVal, &done))
                 return false;
             if (done)
                 break;
             if (!pairVal.isObject()) {
@@ -533,17 +533,17 @@ MapObject::is(HandleValue v)
 
 bool
 MapObject::is(HandleObject o)
 {
     return o->hasClass(&class_) && o->as<MapObject>().getPrivate();
 }
 
 #define ARG0_KEY(cx, args, key)                                               \
-    AutoHashableValueRooter key(cx);                                          \
+    Rooted<HashableValue> key(cx);                                          \
     if (args.length() > 0 && !key.setValue(cx, args[0]))                      \
         return false
 
 ValueMap&
 MapObject::extract(HandleObject o)
 {
     MOZ_ASSERT(o->hasClass(&MapObject::class_));
     return *o->as<MapObject>().getData();
@@ -581,17 +581,17 @@ MapObject::size(JSContext* cx, unsigned 
     return CallNonGenericMethod<MapObject::is, MapObject::size_impl>(cx, args);
 }
 
 bool
 MapObject::get(JSContext* cx, HandleObject obj,
                HandleValue key, MutableHandleValue rval)
 {
     ValueMap& map = extract(obj);
-    AutoHashableValueRooter k(cx);
+    Rooted<HashableValue> k(cx);
 
     if (!k.setValue(cx, key))
         return false;
 
     if (ValueMap::Entry* p = map.get(k))
         rval.set(p->value);
     else
         rval.setUndefined();
@@ -612,17 +612,17 @@ MapObject::get(JSContext* cx, unsigned a
     CallArgs args = CallArgsFromVp(argc, vp);
     return CallNonGenericMethod<MapObject::is, MapObject::get_impl>(cx, args);
 }
 
 bool
 MapObject::has(JSContext* cx, HandleObject obj, HandleValue key, bool* rval)
 {
     ValueMap& map = extract(obj);
-    AutoHashableValueRooter k(cx);
+    Rooted<HashableValue> k(cx);
 
     if (!k.setValue(cx, key))
         return false;
 
     *rval = map.has(k);
     return true;
 }
 
@@ -652,33 +652,33 @@ MapObject::set_impl(JSContext* cx, CallA
 
     ValueMap& map = extract(args);
     ARG0_KEY(cx, args, key);
     RelocatableValue rval(args.get(1));
     if (!map.put(key, rval)) {
         ReportOutOfMemory(cx);
         return false;
     }
-    WriteBarrierPost(cx->runtime(), &map, key.get());
+    WriteBarrierPost(cx->runtime(), &map, key.value());
     args.rval().set(args.thisv());
     return true;
 }
 
 bool
 MapObject::set(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     return CallNonGenericMethod<MapObject::is, MapObject::set_impl>(cx, args);
 }
 
 bool
 MapObject::delete_(JSContext *cx, HandleObject obj, HandleValue key, bool *rval)
 {
     ValueMap &map = extract(obj);
-    AutoHashableValueRooter k(cx);
+    Rooted<HashableValue> k(cx);
 
     if (!k.setValue(cx, key))
         return false;
 
     if (!map.remove(k, rval)) {
         ReportOutOfMemory(cx);
         return false;
     }
@@ -1053,25 +1053,25 @@ SetObject::keys(JSContext* cx, HandleObj
 
 bool
 SetObject::add(JSContext* cx, HandleObject obj, HandleValue k)
 {
     ValueSet* set = obj->as<SetObject>().getData();
     if (!set)
         return false;
 
-    AutoHashableValueRooter key(cx);
+    Rooted<HashableValue> key(cx);
     if (!key.setValue(cx, k))
         return false;
 
     if (!set->put(key)) {
         ReportOutOfMemory(cx);
         return false;
     }
-    WriteBarrierPost(cx->runtime(), set, key.get());
+    WriteBarrierPost(cx->runtime(), set, key.value());
     return true;
 }
 
 SetObject*
 SetObject::create(JSContext* cx)
 {
     SetObject* obj = NewBuiltinClassInstance<SetObject>(cx);
     if (!obj)
@@ -1129,17 +1129,17 @@ SetObject::construct(JSContext* cx, unsi
         RootedValue setVal(cx, ObjectValue(*obj));
         FastInvokeGuard fig(cx, adderVal);
         InvokeArgs& args2 = fig.args();
 
         RootedValue keyVal(cx);
         ForOfIterator iter(cx);
         if (!iter.init(args[0]))
             return false;
-        AutoHashableValueRooter key(cx);
+        Rooted<HashableValue> key(cx);
         ValueSet* set = obj->getData();
         while (true) {
             bool done;
             if (!iter.next(&keyVal, &done))
                 return false;
             if (done)
                 break;
 
@@ -1237,17 +1237,17 @@ SetObject::has_impl(JSContext* cx, CallA
 }
 
 bool
 SetObject::has(JSContext *cx, HandleObject obj, HandleValue key, bool *rval)
 {
     MOZ_ASSERT(SetObject::is(obj));
 
     ValueSet &set = extract(obj);
-    AutoHashableValueRooter k(cx);
+    Rooted<HashableValue> k(cx);
 
     if (!k.setValue(cx, key))
         return false;
 
     *rval = set.has(k);
     return true;
 }
 
@@ -1264,17 +1264,17 @@ SetObject::add_impl(JSContext* cx, CallA
     MOZ_ASSERT(is(args.thisv()));
 
     ValueSet& set = extract(args);
     ARG0_KEY(cx, args, key);
     if (!set.put(key)) {
         ReportOutOfMemory(cx);
         return false;
     }
-    WriteBarrierPost(cx->runtime(), &set, key.get());
+    WriteBarrierPost(cx->runtime(), &set, key.value());
     args.rval().set(args.thisv());
     return true;
 }
 
 bool
 SetObject::add(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
@@ -1282,17 +1282,17 @@ SetObject::add(JSContext* cx, unsigned a
 }
 
 bool
 SetObject::delete_(JSContext *cx, HandleObject obj, HandleValue key, bool *rval)
 {
     MOZ_ASSERT(SetObject::is(obj));
 
     ValueSet &set = extract(obj);
-    AutoHashableValueRooter k(cx);
+    Rooted<HashableValue> k(cx);
 
     if (!k.setValue(cx, key))
         return false;
 
     if (!set.remove(k, rval)) {
         ReportOutOfMemory(cx);
         return false;
     }
--- a/js/src/builtin/MapObject.h
+++ b/js/src/builtin/MapObject.h
@@ -17,17 +17,18 @@ namespace js {
 /*
  * Comparing two ropes for equality can fail. The js::HashTable template
  * requires infallible hash() and match() operations. Therefore we require
  * all values to be converted to hashable form before being used as a key
  * in a Map or Set object.
  *
  * All values except ropes are hashable as-is.
  */
-class HashableValue {
+class HashableValue : public JS::Traceable
+{
     PreBarrieredValue value;
 
   public:
     struct Hasher {
         typedef HashableValue Lookup;
         static HashNumber hash(const Lookup& v) { return v.hash(); }
         static bool match(const HashableValue& k, const Lookup& l) { return k == l; }
         static bool isEmpty(const HashableValue& v) { return v.value.isMagic(JS_HASH_KEY_EMPTY); }
@@ -36,44 +37,31 @@ class HashableValue {
 
     HashableValue() : value(UndefinedValue()) {}
 
     bool setValue(JSContext* cx, HandleValue v);
     HashNumber hash() const;
     bool operator==(const HashableValue& other) const;
     HashableValue mark(JSTracer* trc) const;
     Value get() const { return value.get(); }
+
+    static void trace(HashableValue* value, JSTracer* trc) {
+        TraceEdge(trc, &value->value, "HashableValue");
+    }
 };
 
-class AutoHashableValueRooter : private JS::AutoGCRooter
-{
+template <>
+class RootedBase<HashableValue> {
   public:
-    explicit AutoHashableValueRooter(JSContext* cx
-                                     MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
-        : JS::AutoGCRooter(cx, HASHABLEVALUE)
-        {
-            MOZ_GUARD_OBJECT_NOTIFIER_INIT;
-        }
-
     bool setValue(JSContext* cx, HandleValue v) {
-        return value.setValue(cx, v);
+        return static_cast<JS::Rooted<HashableValue>*>(this)->get().setValue(cx, v);
     }
-
-    operator const HashableValue & () {
-        return value;
+    Value value() const {
+        return static_cast<const JS::Rooted<HashableValue>*>(this)->get().get();
     }
-
-    Value get() const { return value.get(); }
-
-    friend void JS::AutoGCRooter::trace(JSTracer* trc);
-    void trace(JSTracer* trc);
-
-  private:
-    HashableValue value;
-    MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
 template <class Key, class Value, class OrderedHashPolicy, class AllocPolicy>
 class OrderedHashMap;
 
 template <class T, class OrderedHashPolicy, class AllocPolicy>
 class OrderedHashSet;
 
--- a/js/src/gc/RootMarking.cpp
+++ b/js/src/gc/RootMarking.cpp
@@ -138,22 +138,16 @@ AutoGCRooter::trace(JSTracer* trc)
       }
 
       case SCRIPTVECTOR: {
         AutoScriptVector::VectorImpl& vector = static_cast<AutoScriptVector*>(this)->vector;
         TraceRootRange(trc, vector.length(), vector.begin(), "js::AutoScriptVector.vector");
         return;
       }
 
-      case HASHABLEVALUE: {
-        AutoHashableValueRooter* rooter = static_cast<AutoHashableValueRooter*>(this);
-        rooter->trace(trc);
-        return;
-      }
-
       case IONMASM: {
         static_cast<js::jit::MacroAssembler::AutoRooter*>(this)->masm()->trace(trc);
         return;
       }
 
       case WRAPPER: {
         /*
          * We need to use TraceManuallyBarrieredEdge here because we mark
@@ -205,22 +199,16 @@ AutoGCRooter::traceAllWrappers(JSTracer*
         for (AutoGCRooter* gcr = cx->roots.autoGCRooters_; gcr; gcr = gcr->down) {
             if (gcr->tag_ == WRAPVECTOR || gcr->tag_ == WRAPPER)
                 gcr->trace(trc);
         }
     }
 }
 
 void
-AutoHashableValueRooter::trace(JSTracer* trc)
-{
-    TraceRoot(trc, reinterpret_cast<Value*>(&value), "AutoHashableValueRooter");
-}
-
-void
 StackShape::trace(JSTracer* trc)
 {
     if (base)
         TraceRoot(trc, &base, "StackShape base");
 
     TraceRoot(trc, (jsid*) &propid, "StackShape id");
 
     if ((attrs & JSPROP_GETTER) && rawGetter)
--- a/js/src/jspubtd.h
+++ b/js/src/jspubtd.h
@@ -220,17 +220,16 @@ class JS_PUBLIC_API(AutoGCRooter)
         VALARRAY =     -2, /* js::AutoValueArray */
         PARSER =       -3, /* js::frontend::Parser */
         IDARRAY =      -6, /* js::AutoIdArray */
         VALVECTOR =   -10, /* js::AutoValueVector */
         IDVECTOR =    -11, /* js::AutoIdVector */
         OBJVECTOR =   -14, /* js::AutoObjectVector */
         SCRIPTVECTOR =-16, /* js::AutoScriptVector */
         NAMEVECTOR =  -17, /* js::AutoNameVector */
-        HASHABLEVALUE=-18, /* js::HashableValue */
         IONMASM =     -19, /* js::jit::MacroAssembler */
         WRAPVECTOR =  -20, /* js::AutoWrapperVector */
         WRAPPER =     -21, /* js::AutoWrapperRooter */
         JSONPARSER =  -25, /* js::JSONParser */
         CUSTOM =      -26  /* js::CustomAutoRooter */
     };
 
     static ptrdiff_t GetTag(const Value& value) { return VALVECTOR; }