Bug 707313 - use special non-NULL next for weak maps not in the list. r=jimb
authorAndrew McCreight <amccreight@mozilla.com>
Fri, 06 Jan 2012 11:30:09 -0800
changeset 85181 ebd93b75749b5002faa67194b1b58a12d8a8c04e
parent 85180 f0eab7fd20af5570670f3f048846a5536cac0cfe
child 85182 af4f9dd24993d9aa5e3eda3437f6cc7490de674f
push id805
push userakeybl@mozilla.com
push dateWed, 01 Feb 2012 18:17:35 +0000
treeherdermozilla-aurora@6fb3bf232436 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimb
bugs707313
milestone12.0a1
Bug 707313 - use special non-NULL next for weak maps not in the list. r=jimb
js/src/jsgc.cpp
js/src/jsweakmap.cpp
js/src/jsweakmap.h
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -2851,17 +2851,17 @@ MarkAndSweep(JSContext *cx, JSGCInvocati
     JSRuntime *rt = cx->runtime;
     rt->gcNumber++;
 
     /* Clear gcIsNeeded now, when we are about to start a normal GC cycle. */
     rt->gcIsNeeded = false;
     rt->gcTriggerCompartment = NULL;
 
     /* Reset weak map list. */
-    rt->gcWeakMapList = NULL;
+    WeakMapBase::resetWeakMapList(rt);
 
     /* Reset malloc counter. */
     rt->resetGCMallocBytes();
 
     AutoUnlockGC unlock(rt);
 
     GCMarker gcmarker(cx);
     JS_ASSERT(IS_GC_MARKING_TRACER(&gcmarker));
--- a/js/src/jsweakmap.cpp
+++ b/js/src/jsweakmap.cpp
@@ -81,16 +81,30 @@ WeakMapBase::sweepAll(JSTracer *tracer)
 void
 WeakMapBase::traceAllMappings(WeakMapTracer *tracer)
 {
     JSRuntime *rt = tracer->context->runtime;
     for (WeakMapBase *m = rt->gcWeakMapList; m; m = m->next)
         m->traceMappings(tracer);
 }
 
+void
+WeakMapBase::resetWeakMapList(JSRuntime *rt)
+{
+    JS_ASSERT(WeakMapNotInList != NULL);
+
+    WeakMapBase *m = rt->gcWeakMapList;
+    rt->gcWeakMapList = NULL;
+    while (m) {
+        WeakMapBase *n = m->next;
+        m->next = WeakMapNotInList;
+        m = n;
+    }
+}
+
 } /* namespace js */
 
 typedef WeakMap<HeapPtr<JSObject>, HeapValue> ObjectValueMap;
 
 static ObjectValueMap *
 GetObjectMap(JSObject *obj)
 {
     JS_ASSERT(obj->isWeakMap());
--- a/js/src/jsweakmap.h
+++ b/js/src/jsweakmap.h
@@ -101,33 +101,41 @@ namespace js {
 // A policy template holding default marking algorithms for common type combinations. This
 // provides default types for WeakMap's MarkPolicy template parameter.
 template <class Type> class DefaultMarkPolicy;
 
 // A policy template holding default tracing algorithms for common type combinations. This
 // provides default types for WeakMap's TracePolicy template parameter.
 template <class Key, class Value> class DefaultTracePolicy;
 
+// The value for the next pointer for maps not in the map list.
+static WeakMapBase * const WeakMapNotInList = reinterpret_cast<WeakMapBase *>(1);
+
 // Common base class for all WeakMap specializations. The collector uses this to call
 // their markIteratively and sweep methods.
 class WeakMapBase {
   public:
-    WeakMapBase(JSObject *memOf) : memberOf(memOf), next(NULL) { }
+    WeakMapBase(JSObject *memOf) : memberOf(memOf), next(WeakMapNotInList) { }
     virtual ~WeakMapBase() { }
 
     void trace(JSTracer *tracer) {
         if (IS_GC_MARKING_TRACER(tracer)) {
             // We don't do anything with a WeakMap at trace time. Rather, we wait until as
             // many keys as possible have been marked, and add ourselves to the list of
             // known-live WeakMaps to be scanned in the iterative marking phase, by
             // markAllIteratively.
             JS_ASSERT(!tracer->eagerlyTraceWeakMaps);
-            JSRuntime *rt = tracer->context->runtime;
-            next = rt->gcWeakMapList;
-            rt->gcWeakMapList = this;
+
+            // Add ourselves to the list if we are not already in the list. We can already
+            // be in the list if the weak map is marked more than once due delayed marking.
+            if (next == WeakMapNotInList) {
+                JSRuntime *rt = tracer->context->runtime;
+                next = rt->gcWeakMapList;
+                rt->gcWeakMapList = this;
+            }
         } else {
             // If we're not actually doing garbage collection, the keys won't be marked
             // nicely as needed by the true ephemeral marking algorithm --- custom tracers
             // such as the cycle collector must use their own means for cycle detection.
             // So here we do a conservative approximation: pretend all keys are live.
             if (tracer->eagerlyTraceWeakMaps)
                 nonMarkingTrace(tracer);
         }
@@ -143,30 +151,36 @@ class WeakMapBase {
 
     // Remove entries whose keys are dead from all weak maps marked as live in this
     // garbage collection.
     static void sweepAll(JSTracer *tracer);
 
     // Trace all delayed weak map bindings. Used by the cycle collector.
     static void traceAllMappings(WeakMapTracer *tracer);
 
+    // Remove everything from the live weak map list.
+    static void resetWeakMapList(JSRuntime *rt);
+
   protected:
     // Instance member functions called by the above. Instantiations of WeakMap override
     // these with definitions appropriate for their Key and Value types.
     virtual void nonMarkingTrace(JSTracer *tracer) = 0;
     virtual bool markIteratively(JSTracer *tracer) = 0;
     virtual void sweep(JSTracer *tracer) = 0;
     virtual void traceMappings(WeakMapTracer *tracer) = 0;
 
     // Object that this weak map is part of, if any.
     JSObject *memberOf;
 
   private:
     // Link in a list of WeakMaps to mark iteratively and sweep in this garbage
-    // collection, headed by JSRuntime::gcWeakMapList.
+    // collection, headed by JSRuntime::gcWeakMapList. The last element of the list
+    // has NULL as its next. Maps not in the list have WeakMapNotInList as their
+    // next.  We must distinguish these cases to avoid creating infinite lists
+    // when a weak map gets traced twice due to delayed marking.
     WeakMapBase *next;
 };
 
 template <class Key, class Value,
           class HashPolicy = DefaultHasher<Key>,
           class KeyMarkPolicy = DefaultMarkPolicy<Key>,
           class ValueMarkPolicy = DefaultMarkPolicy<Value>,
           class TracePolicy = DefaultTracePolicy<Key, Value> >