From 555fb4ae35a549e50b89fead7c1db93c39a0f718 Mon Sep 17 00:00:00 2001
From: Luke Wagner <luke@mozilla.com>
Date: Thu, 1 Nov 2012 21:27:07 -0700
Subject: [PATCH] Bug 805294 - Don't use the RegExpShared cache to track all
 live RegExpShareds (r=billm)

---
 js/src/jscompartment.cpp   |  6 +++++-
 js/src/vm/RegExpObject.cpp | 26 ++++++++++++++++++++------
 js/src/vm/RegExpObject.h   | 25 ++++++++++++++++++++++---
 3 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/js/src/jscompartment.cpp b/js/src/jscompartment.cpp
index b7e173c4b976..4d57f5d9f8fc 100644
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -561,7 +561,11 @@ JSCompartment::sweep(FreeOp *fop, bool releaseTypes)
             ionCompartment_->sweep(fop);
 #endif
 
-        /* JIT code can hold references on RegExpShared, so sweep regexps after clearing code. */
+        /*
+         * JIT code increments activeUseCount for any RegExpShared used by jit
+         * code for the lifetime of the JIT script. Thus, we must perform
+         * sweeping after clearing jit code.
+         */
         regExps.sweep(rt);
     }
 
diff --git a/js/src/vm/RegExpObject.cpp b/js/src/vm/RegExpObject.cpp
index 5c78ce881883..059aff088320 100644
--- a/js/src/vm/RegExpObject.cpp
+++ b/js/src/vm/RegExpObject.cpp
@@ -519,18 +519,19 @@ RegExpShared::execute(JSContext *cx, StableCharPtr chars, size_t length, size_t
 /* RegExpCompartment */
 
 RegExpCompartment::RegExpCompartment(JSRuntime *rt)
-  : map_(rt)
+  : map_(rt), inUse_(rt)
 {}
 
 RegExpCompartment::~RegExpCompartment()
 {
-    map_.empty();
+    JS_ASSERT(map_.empty());
+    JS_ASSERT(inUse_.empty());
 }
 
 bool
 RegExpCompartment::init(JSContext *cx)
 {
-    if (!map_.init()) {
+    if (!map_.init() || !inUse_.init()) {
         js_ReportOutOfMemory(cx);
         return false;
     }
@@ -538,12 +539,19 @@ RegExpCompartment::init(JSContext *cx)
     return true;
 }
 
+/* See the comment on RegExpShared lifetime in RegExpObject.h. */
 void
 RegExpCompartment::sweep(JSRuntime *rt)
 {
-    for (Map::Enum e(map_); !e.empty(); e.popFront()) {
-        /* See the comment on RegExpShared lifetime in RegExpObject.h. */
-        RegExpShared *shared = e.front().value;
+#ifdef DEBUG
+    for (Map::Range r = map_.all(); !r.empty(); r.popFront())
+        JS_ASSERT(inUse_.has(r.front().value));
+#endif
+
+    map_.clear();
+
+    for (PendingSet::Enum e(inUse_); !e.empty(); e.popFront()) {
+        RegExpShared *shared = e.front();
         if (shared->activeUseCount == 0 && shared->gcNumberWhenUsed < rt->gcStartNumber) {
             js_delete(shared);
             e.removeFront();
@@ -575,6 +583,12 @@ RegExpCompartment::get(JSContext *cx, JSAtom *keyAtom, JSAtom *source, RegExpFla
         return false;
     }
 
+    if (!inUse_.put(shared)) {
+        map_.remove(key);
+        js_ReportOutOfMemory(cx);
+        return false;
+    }
+
     /*
      * Since 'error' deletes 'shared', only guard 'shared' on success. This is
      * safe since 'shared' cannot be deleted by GC until after the call to
diff --git a/js/src/vm/RegExpObject.h b/js/src/vm/RegExpObject.h
index af5b3bd45f0e..f605d5dd4459 100644
--- a/js/src/vm/RegExpObject.h
+++ b/js/src/vm/RegExpObject.h
@@ -140,9 +140,9 @@ class RegExpCode
 
 /*
  * A RegExpShared is the compiled representation of a regexp. A RegExpShared is
- * pointed to by potentially multiple RegExpObjects. Additionally, C++ code may
- * have pointers to RegExpShareds on the stack. The RegExpShareds are tracked in
- * a RegExpCompartment hashtable, and most are destroyed on every GC.
+ * potentially pointed to by multiple RegExpObjects. Additionally, C++ code may
+ * have pointers to RegExpShareds on the stack. The RegExpShareds are kept in a
+ * cache so that they can be reused when compiling the same regex string.
  *
  * During a GC, the trace hook for RegExpObject clears any pointers to
  * RegExpShareds so that there will be no dangling pointers when they are
@@ -160,6 +160,13 @@ class RegExpCode
  *
  * The activeUseCount and gcNumberWhenUsed fields are used to track these
  * conditions.
+ *
+ * There are two tables used to track RegExpShareds. map_ implements the cache
+ * and is cleared on every GC. inUse_ logically owns all RegExpShareds in the
+ * compartment and attempts to delete all RegExpShareds that aren't kept alive
+ * by the above conditions on every GC sweep phase. It is necessary to use two
+ * separate tables since map_ *must* be fully cleared on each GC since the Key
+ * points to a JSAtom that can become garbage.
  */
 class RegExpShared
 {
@@ -251,9 +258,21 @@ class RegExpCompartment
         }
     };
 
+    /*
+     * Cache to reuse RegExpShareds with the same source/flags/etc. The cache
+     * is entirely cleared on each GC.
+     */
     typedef HashMap<Key, RegExpShared *, Key, RuntimeAllocPolicy> Map;
     Map map_;
 
+    /*
+     * The set of all RegExpShareds in the compartment. On every GC, every
+     * RegExpShared that is not actively being used is deleted and removed from
+     * the set.
+     */
+    typedef HashSet<RegExpShared *, DefaultHasher<RegExpShared*>, RuntimeAllocPolicy> PendingSet;
+    PendingSet inUse_;
+
     bool get(JSContext *cx, JSAtom *key, JSAtom *source, RegExpFlag flags, Type type,
              RegExpGuard *g);