Bug 1318384 - Remove barriers on Zone typed object descriptors set to avoid reviving compartments r=billm
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 23 Nov 2016 10:15:05 +0000
changeset 323933 b1a5a845e2940eb2f4abb4f60c73721ec15662bc
parent 323932 ace95a181f29d8bafa390026d82b725103cc00d0
child 323934 4772a0f4bed2e9f0cce209eec7560ede79097398
push id24
push usermaklebus@msu.edu
push dateTue, 20 Dec 2016 03:11:33 +0000
reviewersbillm
bugs1318384
milestone53.0a1
Bug 1318384 - Remove barriers on Zone typed object descriptors set to avoid reviving compartments r=billm
js/public/GCPolicyAPI.h
js/public/TracingAPI.h
js/src/builtin/TypedObject.cpp
js/src/gc/Zone.cpp
js/src/gc/Zone.h
js/src/jsgc.cpp
--- a/js/public/GCPolicyAPI.h
+++ b/js/public/GCPolicyAPI.h
@@ -114,16 +114,21 @@ template <> struct GCPolicy<uint64_t> : 
 template <typename T>
 struct GCPointerPolicy
 {
     static T initial() { return nullptr; }
     static void trace(JSTracer* trc, T* vp, const char* name) {
         if (*vp)
             js::UnsafeTraceManuallyBarrieredEdge(trc, vp, name);
     }
+    static bool needsSweep(T* vp) {
+        if (*vp)
+            return js::gc::IsAboutToBeFinalizedUnbarriered(vp);
+        return false;
+    }
 };
 template <> struct GCPolicy<JS::Symbol*> : public GCPointerPolicy<JS::Symbol*> {};
 template <> struct GCPolicy<JSAtom*> : public GCPointerPolicy<JSAtom*> {};
 template <> struct GCPolicy<JSFunction*> : public GCPointerPolicy<JSFunction*> {};
 template <> struct GCPolicy<JSObject*> : public GCPointerPolicy<JSObject*> {};
 template <> struct GCPolicy<JSScript*> : public GCPointerPolicy<JSScript*> {};
 template <> struct GCPolicy<JSString*> : public GCPointerPolicy<JSString*> {};
 
--- a/js/public/TracingAPI.h
+++ b/js/public/TracingAPI.h
@@ -380,16 +380,24 @@ namespace js {
 //
 // This method does not check if |*edgep| is non-null before tracing through
 // it, so callers must check any nullable pointer before calling this method.
 template <typename T>
 extern JS_PUBLIC_API(void)
 UnsafeTraceManuallyBarrieredEdge(JSTracer* trc, T* edgep, const char* name);
 
 namespace gc {
+
 // Return true if the given edge is not live and is about to be swept.
 template <typename T>
 extern JS_PUBLIC_API(bool)
 EdgeNeedsSweep(JS::Heap<T>* edgep);
+
+// Not part of the public API, but declared here so we can use it in GCPolicy
+// which is.
+template <typename T>
+bool
+IsAboutToBeFinalizedUnbarriered(T* thingp);
+
 } // namespace gc
 } // namespace js
 
 #endif /* js_TracingAPI_h */
--- a/js/src/builtin/TypedObject.cpp
+++ b/js/src/builtin/TypedObject.cpp
@@ -647,17 +647,17 @@ ArrayMetaTypeDescr::create(JSContext* cx
     obj->initReservedSlot(JS_DESCR_SLOT_TYPROTO, ObjectValue(*prototypeObj));
 
     if (!LinkConstructorAndPrototype(cx, obj, prototypeObj))
         return nullptr;
 
     if (!CreateTraceList(cx, obj))
         return nullptr;
 
-    if (!cx->zone()->typeDescrObjects.put(obj)) {
+    if (!cx->zone()->addTypeDescrObject(cx, obj)) {
         ReportOutOfMemory(cx);
         return nullptr;
     }
 
     return obj;
 }
 
 bool
@@ -988,18 +988,18 @@ StructMetaTypeDescr::create(JSContext* c
     descr->initReservedSlot(JS_DESCR_SLOT_TYPROTO, ObjectValue(*prototypeObj));
 
     if (!LinkConstructorAndPrototype(cx, descr, prototypeObj))
         return nullptr;
 
     if (!CreateTraceList(cx, descr))
         return nullptr;
 
-    if (!cx->zone()->typeDescrObjects.put(descr) ||
-        !cx->zone()->typeDescrObjects.put(fieldTypeVec))
+    if (!cx->zone()->addTypeDescrObject(cx, descr) ||
+        !cx->zone()->addTypeDescrObject(cx, fieldTypeVec))
     {
         ReportOutOfMemory(cx);
         return nullptr;
     }
 
     return descr;
 }
 
@@ -1160,20 +1160,18 @@ DefineSimpleTypeDescr(JSContext* cx,
 
     RootedValue descrValue(cx, ObjectValue(*descr));
     if (!DefineProperty(cx, module, className, descrValue, nullptr, nullptr, 0))
         return false;
 
     if (!CreateTraceList(cx, descr))
         return false;
 
-    if (!cx->zone()->typeDescrObjects.put(descr)) {
-        ReportOutOfMemory(cx);
+    if (!cx->zone()->addTypeDescrObject(cx, descr))
         return false;
-    }
 
     return true;
 }
 
 ///////////////////////////////////////////////////////////////////////////
 
 template<typename T>
 static JSObject*
--- a/js/src/gc/Zone.cpp
+++ b/js/src/gc/Zone.cpp
@@ -364,16 +364,31 @@ Zone::clearTables()
 }
 
 void
 Zone::fixupAfterMovingGC()
 {
     fixupInitialShapeTable();
 }
 
+bool
+Zone::addTypeDescrObject(JSContext* cx, HandleObject obj)
+{
+    // Type descriptor objects are always tenured so we don't need post barriers
+    // on the set.
+    MOZ_ASSERT(!IsInsideNursery(obj));
+
+    if (!typeDescrObjects.put(obj)) {
+        ReportOutOfMemory(cx);
+        return false;
+    }
+
+    return true;
+}
+
 ZoneList::ZoneList()
   : head(nullptr), tail(nullptr)
 {}
 
 ZoneList::ZoneList(Zone* zone)
   : head(zone), tail(zone)
 {
     MOZ_RELEASE_ASSERT(!zone->isOnList());
--- a/js/src/gc/Zone.h
+++ b/js/src/gc/Zone.h
@@ -350,21 +350,26 @@ struct Zone : public JS::shadow::Zone,
     //
     // This is used during GC while calculating zone groups to record edges that
     // can't be determined by examining this zone by itself.
     ZoneSet gcZoneGroupEdges;
 
     // Keep track of all TypeDescr and related objects in this compartment.
     // This is used by the GC to trace them all first when compacting, since the
     // TypedObject trace hook may access these objects.
-    using TypeDescrObjectSet = js::GCHashSet<js::HeapPtr<JSObject*>,
-                                             js::MovableCellHasher<js::HeapPtr<JSObject*>>,
+    //
+    // There are no barriers here - the set contains only tenured objects so no
+    // post-barrier is required, and these are weak references so no pre-barrier
+    // is required.
+    using TypeDescrObjectSet = js::GCHashSet<JSObject*,
+                                             js::MovableCellHasher<JSObject*>,
                                              js::SystemAllocPolicy>;
     JS::WeakCache<TypeDescrObjectSet> typeDescrObjects;
 
+    bool addTypeDescrObject(JSContext* cx, HandleObject obj);
 
     // Malloc counter to measure memory pressure for GC scheduling. It runs from
     // gcMaxMallocBytes down to zero. This counter should be used only when it's
     // not possible to know the size of a free.
     mozilla::Atomic<ptrdiff_t, mozilla::ReleaseAcquire> gcMallocBytes;
 
     // GC trigger threshold for allocations on the C heap.
     size_t gcMaxMallocBytes;
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -2401,17 +2401,17 @@ ForegroundUpdateKinds(AllocKinds kinds)
     return result;
 }
 
 void
 GCRuntime::updateTypeDescrObjects(MovingTracer* trc, Zone* zone)
 {
     zone->typeDescrObjects.sweep();
     for (auto r = zone->typeDescrObjects.all(); !r.empty(); r.popFront())
-        UpdateCellPointers(trc, r.front().get());
+        UpdateCellPointers(trc, r.front());
 }
 
 void
 GCRuntime::updateCellPointers(MovingTracer* trc, Zone* zone, AllocKinds kinds, size_t bgTaskCount)
 {
     AllocKinds fgKinds = bgTaskCount == 0 ? kinds : ForegroundUpdateKinds(kinds);
     AllocKinds bgKinds = kinds - fgKinds;