Bug 820124, Part 1/2 - Use RegExpShared for lazy RegExpStatics. r=dvander
☠☠ backed out by c55e74c41184 ☠ ☠
authorSean Stangl <sstangl@mozilla.com>
Fri, 14 Dec 2012 16:07:35 -0800
changeset 126181 4a7071b920691e2ac545cb59a3fc074d4b904640
parent 126180 9afe210481680c275666953c9830fddf2366fc95
child 126182 038194a2ffc38c3d893cfe8e8c81a987a149e29f
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs820124
milestone20.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 820124, Part 1/2 - Use RegExpShared for lazy RegExpStatics. r=dvander
js/src/builtin/RegExp.cpp
js/src/gc/Marking.cpp
js/src/gc/Marking.h
js/src/gc/RootMarking.cpp
js/src/vm/RegExpObject.cpp
js/src/vm/RegExpObject.h
js/src/vm/RegExpStatics-inl.h
js/src/vm/RegExpStatics.cpp
js/src/vm/RegExpStatics.h
--- a/js/src/builtin/RegExp.cpp
+++ b/js/src/builtin/RegExp.cpp
@@ -109,29 +109,29 @@ js::CreateRegExpMatchResult(JSContext *c
 {
     Rooted<JSStableString*> input(cx, string->ensureStable(cx));
     if (!input)
         return false;
     return CreateRegExpMatchResult(cx, input, input->chars(), input->length(), matches, rval);
 }
 
 RegExpRunStatus
-ExecuteRegExpImpl(JSContext *cx, RegExpStatics *res, RegExpShared &re, RegExpObject &regexp,
+ExecuteRegExpImpl(JSContext *cx, RegExpStatics *res, RegExpShared &re,
                   JSLinearString *input, StableCharPtr chars, size_t length,
                   size_t *lastIndex, MatchConduit &matches)
 {
     RegExpRunStatus status;
 
     /* Switch between MatchOnly and IncludeSubpatterns modes. */
     if (matches.isPair) {
         size_t lastIndex_orig = *lastIndex;
         /* Only one MatchPair slot provided: execute short-circuiting regexp. */
         status = re.executeMatchOnly(cx, chars, length, lastIndex, *matches.u.pair);
         if (status == RegExpRunStatus_Success && res)
-            res->updateLazily(cx, input, &regexp, lastIndex_orig);
+            res->updateLazily(cx, input, &re, lastIndex_orig);
     } else {
         /* Vector of MatchPairs provided: execute full regexp. */
         status = re.execute(cx, chars, length, lastIndex, *matches.u.pairs);
         if (status == RegExpRunStatus_Success && res)
             res->updateFromMatchPairs(cx, input, *matches.u.pairs);
     }
 
     return status;
@@ -146,17 +146,17 @@ js::ExecuteRegExpLegacy(JSContext *cx, R
     RegExpGuard shared;
     if (!reobj.getShared(cx, &shared))
         return false;
 
     ScopedMatchPairs matches(&cx->tempLifoAlloc());
     MatchConduit conduit(&matches);
 
     RegExpRunStatus status =
-        ExecuteRegExpImpl(cx, res, *shared, reobj, input, chars, length, lastIndex, conduit);
+        ExecuteRegExpImpl(cx, res, *shared, input, chars, length, lastIndex, conduit);
 
     if (status == RegExpRunStatus_Error)
         return false;
 
     if (status == RegExpRunStatus_Success_NotFound) {
         /* ExecuteRegExp() previously returned an array or null. */
         rval->setNull();
         return true;
@@ -586,17 +586,17 @@ js::ExecuteRegExp(JSContext *cx, HandleO
     if (i < 0 || i > length) {
         reobj->zeroLastIndex();
         return RegExpRunStatus_Success_NotFound;
     }
 
     /* Steps 8-21. */
     size_t lastIndexInt(i);
     RegExpRunStatus status =
-        ExecuteRegExpImpl(cx, res, *re, *reobj, stableInput, chars, length, &lastIndexInt, matches);
+        ExecuteRegExpImpl(cx, res, *re, stableInput, chars, length, &lastIndexInt, matches);
 
     if (status == RegExpRunStatus_Error)
         return RegExpRunStatus_Error;
 
     /* Step 11 (with sticky extension). */
     if (re->global() || (status == RegExpRunStatus_Success && re->sticky())) {
         if (status == RegExpRunStatus_Success_NotFound)
             reobj->zeroLastIndex();
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -327,17 +327,16 @@ Is##base##AboutToBeFinalized(Encapsulate
 DeclMarkerImpl(BaseShape, BaseShape)
 DeclMarkerImpl(BaseShape, UnownedBaseShape)
 DeclMarkerImpl(IonCode, ion::IonCode)
 DeclMarkerImpl(Object, ArgumentsObject)
 DeclMarkerImpl(Object, DebugScopeObject)
 DeclMarkerImpl(Object, GlobalObject)
 DeclMarkerImpl(Object, JSObject)
 DeclMarkerImpl(Object, JSFunction)
-DeclMarkerImpl(Object, RegExpObject)
 DeclMarkerImpl(Object, ScopeObject)
 DeclMarkerImpl(Script, JSScript)
 DeclMarkerImpl(Shape, Shape)
 DeclMarkerImpl(String, JSAtom)
 DeclMarkerImpl(String, JSString)
 DeclMarkerImpl(String, JSFlatString)
 DeclMarkerImpl(String, JSLinearString)
 DeclMarkerImpl(String, PropertyName)
--- a/js/src/gc/Marking.h
+++ b/js/src/gc/Marking.h
@@ -92,17 +92,16 @@ bool Is##base##AboutToBeFinalized(Encaps
 DeclMarker(BaseShape, BaseShape)
 DeclMarker(BaseShape, UnownedBaseShape)
 DeclMarker(IonCode, ion::IonCode)
 DeclMarker(Object, ArgumentsObject)
 DeclMarker(Object, DebugScopeObject)
 DeclMarker(Object, GlobalObject)
 DeclMarker(Object, JSObject)
 DeclMarker(Object, JSFunction)
-DeclMarker(Object, RegExpObject)
 DeclMarker(Object, ScopeObject)
 DeclMarker(Script, JSScript)
 DeclMarker(Shape, Shape)
 DeclMarker(String, JSAtom)
 DeclMarker(String, JSString)
 DeclMarker(String, JSFlatString)
 DeclMarker(String, JSLinearString)
 DeclMarker(String, PropertyName)
--- a/js/src/gc/RootMarking.cpp
+++ b/js/src/gc/RootMarking.cpp
@@ -669,19 +669,16 @@ Shape::Range::AutoRooter::trace(JSTracer
 {
     if (r->cursor)
         MarkShapeRoot(trc, const_cast<Shape**>(&r->cursor), "Shape::Range::AutoRooter");
 }
 
 void
 RegExpStatics::AutoRooter::trace(JSTracer *trc)
 {
-    if (statics->regexp)
-        MarkObjectRoot(trc, reinterpret_cast<JSObject**>(&statics->regexp),
-                       "RegExpStatics::AutoRooter regexp");
     if (statics->matchesInput)
         MarkStringRoot(trc, reinterpret_cast<JSString**>(&statics->matchesInput),
                        "RegExpStatics::AutoRooter matchesInput");
     if (statics->pendingInput)
         MarkStringRoot(trc, reinterpret_cast<JSString**>(&statics->pendingInput),
                        "RegExpStatics::AutoRooter pendingInput");
 }
 
--- a/js/src/vm/RegExpObject.cpp
+++ b/js/src/vm/RegExpObject.cpp
@@ -640,16 +640,28 @@ RegExpShared::executeMatchOnly(JSContext
 
 RegExpCompartment::RegExpCompartment(JSRuntime *rt)
   : map_(rt), inUse_(rt)
 {}
 
 RegExpCompartment::~RegExpCompartment()
 {
     JS_ASSERT(map_.empty());
+
+    /*
+     * RegExpStatics may have prevented a single RegExpShared from
+     * being collected during RegExpCompartment::sweep().
+     */
+    if (!inUse_.empty()) {
+        PendingSet::Enum e(inUse_);
+        RegExpShared *shared = e.front();
+        JS_ASSERT(shared->activeUseCount == 0);
+        js_delete(shared);
+        e.removeFront();
+    }
     JS_ASSERT(inUse_.empty());
 }
 
 bool
 RegExpCompartment::init(JSContext *cx)
 {
     if (!map_.init() || !inUse_.init()) {
         if (cx)
--- a/js/src/vm/RegExpObject.h
+++ b/js/src/vm/RegExpObject.h
@@ -193,46 +193,58 @@ class RegExpShared
 
 /*
  * Extend the lifetime of a given RegExpShared to at least the lifetime of
  * the guard object. See Regular Expression comment at the top.
  */
 class RegExpGuard
 {
     RegExpShared *re_;
+
+  private:
     RegExpGuard(const RegExpGuard &) MOZ_DELETE;
     void operator=(const RegExpGuard &) MOZ_DELETE;
+
   public:
     RegExpGuard() : re_(NULL) {}
     RegExpGuard(RegExpShared &re) : re_(&re) {
         re_->incRef();
     }
+    ~RegExpGuard() { release(); }
+
+  public:
     void init(RegExpShared &re) {
-        JS_ASSERT(!re_);
+        JS_ASSERT(!initialized());
         re_ = &re;
         re_->incRef();
     }
-    ~RegExpGuard() {
-        if (re_)
+    void release() {
+        if (re_) {
             re_->decRef();
+            re_ = NULL;
+        }
     }
+
     bool initialized() const { return !!re_; }
     RegExpShared *re() const { JS_ASSERT(initialized()); return re_; }
     RegExpShared *operator->() { return re(); }
     RegExpShared &operator*() { return *re(); }
 };
 
 class RegExpCompartment
 {
     struct Key {
         JSAtom *atom;
         uint16_t flag;
+
         Key() {}
         Key(JSAtom *atom, RegExpFlag flag)
-          : atom(atom), flag(flag) {}
+          : atom(atom), flag(flag)
+        { }
+
         typedef Key Lookup;
         static HashNumber hash(const Lookup &l) {
             return DefaultHasher<JSAtom *>::hash(l.atom) ^ (l.flag << 1);
         }
         static bool match(Key l, Key r) {
             return l.atom == r.atom && l.flag == r.flag;
         }
     };
--- a/js/src/vm/RegExpStatics-inl.h
+++ b/js/src/vm/RegExpStatics-inl.h
@@ -22,25 +22,16 @@ js::GlobalObject::getRegExpStatics() con
 }
 
 inline size_t
 SizeOfRegExpStaticsData(const JSObject *obj, JSMallocSizeOfFun mallocSizeOf)
 {
     return mallocSizeOf(obj->getPrivate());
 }
 
-inline
-RegExpStatics::RegExpStatics()
-  : pendingLazyEvaluation(false),
-    bufferLink(NULL),
-    copied(false)
-{
-    clear();
-}
-
 inline bool
 RegExpStatics::createDependent(JSContext *cx, size_t start, size_t end, Value *out)
 {
     /* Private function: caller must perform lazy evaluation. */
     JS_ASSERT(!pendingLazyEvaluation);
 
     JS_ASSERT(start <= end);
     JS_ASSERT(end <= matchesInput->length());
@@ -226,25 +217,30 @@ RegExpStatics::getRightContext(JSSubStri
 
 inline void
 RegExpStatics::copyTo(RegExpStatics &dst)
 {
     /* Destination buffer has already been reserved by save(). */
     if (!pendingLazyEvaluation)
         dst.matches.initArrayFrom(matches);
 
+    if (regexpGuard.initialized())
+        dst.regexpGuard.init(*regexpGuard);
+    else
+        dst.regexpGuard.release();
+
     dst.matchesInput = matchesInput;
-    dst.regexp = regexp;
     dst.lastIndex = lastIndex;
     dst.pendingInput = pendingInput;
     dst.flags = flags;
     dst.pendingLazyEvaluation = pendingLazyEvaluation;
 
-    JS_ASSERT_IF(pendingLazyEvaluation, regexp);
+    JS_ASSERT_IF(pendingLazyEvaluation, regexpGuard.initialized());
     JS_ASSERT_IF(pendingLazyEvaluation, matchesInput);
+    JS_ASSERT(regexpGuard.initialized() == dst.regexpGuard.initialized());
 }
 
 inline void
 RegExpStatics::aboutToWrite()
 {
     if (bufferLink && !bufferLink->copied) {
         copyTo(*bufferLink);
         bufferLink->copied = true;
@@ -256,38 +252,41 @@ RegExpStatics::restore()
 {
     if (bufferLink->copied)
         bufferLink->copyTo(*this);
     bufferLink = bufferLink->bufferLink;
 }
 
 inline void
 RegExpStatics::updateLazily(JSContext *cx, JSLinearString *input,
-                            RegExpObject *regexp, size_t lastIndex)
+                            RegExpShared *shared, size_t lastIndex)
 {
-    JS_ASSERT(input && regexp);
+    JS_ASSERT(input && shared);
     aboutToWrite();
 
     BarrieredSetPair<JSString, JSLinearString>(cx->compartment,
                                                pendingInput, input,
                                                matchesInput, input);
+    if (regexpGuard.initialized())
+        regexpGuard.release();
+    regexpGuard.init(*shared);
+
+    this->lastIndex = lastIndex;
     pendingLazyEvaluation = true;
-    this->regexp = regexp;
-    this->lastIndex = lastIndex;
 }
 
 inline bool
 RegExpStatics::updateFromMatchPairs(JSContext *cx, JSLinearString *input, MatchPairs &newPairs)
 {
     JS_ASSERT(input);
     aboutToWrite();
 
     /* Unset all lazy state. */
     pendingLazyEvaluation = false;
-    this->regexp = NULL;
+    this->regexpGuard.release();
     this->lastIndex = size_t(-1);
 
     BarrieredSetPair<JSString, JSLinearString>(cx->compartment,
                                                pendingInput, input,
                                                matchesInput, input);
 
     if (!matches.initArrayFrom(newPairs)) {
         js_ReportOutOfMemory(cx);
@@ -296,21 +295,24 @@ RegExpStatics::updateFromMatchPairs(JSCo
 
     return true;
 }
 
 inline void
 RegExpStatics::clear()
 {
     aboutToWrite();
+
+    matches.forgetArray();
+    matchesInput = NULL;
+    regexpGuard.release();
+    lastIndex = size_t(-1);
+    pendingInput = NULL;
     flags = RegExpFlag(0);
-    pendingInput = NULL;
     pendingLazyEvaluation = false;
-    matchesInput = NULL;
-    matches.forgetArray();
 }
 
 inline void
 RegExpStatics::setPendingInput(JSString *newInput)
 {
     aboutToWrite();
     pendingInput = newInput;
 }
@@ -358,18 +360,19 @@ RegExpStatics::reset(JSContext *cx, JSSt
     checkInvariants();
 }
 
 inline void
 RegExpStatics::checkInvariants()
 {
 #ifdef DEBUG
     if (pendingLazyEvaluation) {
-        JS_ASSERT(regexp);
+        JS_ASSERT(regexpGuard.initialized());
         JS_ASSERT(pendingInput);
+        JS_ASSERT(lastIndex != size_t(-1));
         return;
     }
 
     if (matches.empty()) {
         JS_ASSERT(!matchesInput);
         return;
     }
 
--- a/js/src/vm/RegExpStatics.cpp
+++ b/js/src/vm/RegExpStatics.cpp
@@ -69,35 +69,32 @@ RegExpStatics::create(JSContext *cx, Glo
 }
 
 bool
 RegExpStatics::executeLazy(JSContext *cx)
 {
     if (!pendingLazyEvaluation)
         return true;
 
-    JS_ASSERT(regexp);
+    JS_ASSERT(regexpGuard.initialized());
     JS_ASSERT(matchesInput);
     JS_ASSERT(lastIndex != size_t(-1));
 
     /*
      * It is not necessary to call aboutToWrite(): evaluation of
      * implicit copies is safe.
      */
 
     size_t length = matchesInput->length();
     StableCharPtr chars(matchesInput->chars(), length);
 
     /* Execute the full regular expression. */
-    RegExpGuard shared;
-    if (!regexp->getShared(cx, &shared))
-        return false;
-
-    RegExpRunStatus status = shared->execute(cx, chars, length, &this->lastIndex, this->matches);
+    RegExpRunStatus status = regexpGuard->execute(cx, chars, length, &this->lastIndex, this->matches);
     if (status == RegExpRunStatus_Error)
         return false;
 
     /* Unset lazy state and remove rooted values that now have no use. */
     pendingLazyEvaluation = false;
-    regexp = NULL;
+    regexpGuard.release();
+    lastIndex = size_t(-1);
 
     return true;
 }
--- a/js/src/vm/RegExpStatics.h
+++ b/js/src/vm/RegExpStatics.h
@@ -10,43 +10,48 @@
 
 #include "jscntxt.h"
 
 #include "gc/Barrier.h"
 #include "gc/Marking.h"
 #include "js/Vector.h"
 
 #include "vm/MatchPairs.h"
+#include "vm/RegExpObject.h"
 
 namespace js {
 
 class RegExpStatics
 {
     /* The latest RegExp output, set after execution. */
     VectorMatchPairs        matches;
     HeapPtr<JSLinearString> matchesInput;
 
     /* The previous RegExp input, used to resolve lazy state. */
-    HeapPtr<RegExpObject>   regexp;
+    RegExpGuard             regexpGuard;  /* Strong reference to RegExpShared. */
     size_t                  lastIndex;
 
     /* The latest RegExp input, set before execution. */
     HeapPtr<JSString>       pendingInput;
     RegExpFlag              flags;
 
     /*
-     * If true, |matchesInput|, |regexp|, and |lastIndex| may be used
+     * If true, |matchesInput|, |regexpGuard|, and |lastIndex| may be used
      * to replay the last executed RegExp, and |matches| is invalid.
      */
     bool                    pendingLazyEvaluation;
 
     /* Linkage for preserving RegExpStatics during nested RegExp execution. */
     RegExpStatics           *bufferLink;
     bool                    copied;
 
+  public:
+    RegExpStatics() : bufferLink(NULL), copied(false) { clear(); }
+    static JSObject *create(JSContext *cx, GlobalObject *parent);
+
   private:
     bool executeLazy(JSContext *cx);
 
     inline void aboutToWrite();
     inline void copyTo(RegExpStatics &dst);
 
     inline void restore();
     bool save(JSContext *cx, RegExpStatics *buffer) {
@@ -73,24 +78,19 @@ class RegExpStatics
     void markFlagsSet(JSContext *cx);
 
     struct InitBuffer {};
     explicit RegExpStatics(InitBuffer) : bufferLink(NULL), copied(false) {}
 
     friend class PreserveRegExpStatics;
 
   public:
-    inline RegExpStatics();
-
-    static JSObject *create(JSContext *cx, GlobalObject *parent);
-
     /* Mutators. */
-
     inline void updateLazily(JSContext *cx, JSLinearString *input,
-                             RegExpObject *regexp, size_t lastIndex);
+                             RegExpShared *shared, size_t lastIndex);
     inline bool updateFromMatchPairs(JSContext *cx, JSLinearString *input, MatchPairs &newPairs);
     inline void setMultiline(JSContext *cx, bool enabled);
 
     inline void clear();
 
     /* Corresponds to JSAPI functionality to set the pending RegExp input. */
     inline void reset(JSContext *cx, JSString *newInput, bool newMultiline);
 
@@ -113,18 +113,16 @@ class RegExpStatics
     bool matched() const {
         /* Safe: only used by String methods, which do not set lazy mode. */
         JS_ASSERT(!pendingLazyEvaluation);
         JS_ASSERT(matches.pairCount() > 0);
         return matches[0].limit - matches[0].start > 0;
     }
 
     void mark(JSTracer *trc) {
-        if (regexp)
-            gc::MarkObject(trc, &regexp, "res->regexp");
         if (pendingInput)
             MarkString(trc, &pendingInput, "res->pendingInput");
         if (matchesInput)
             MarkString(trc, &matchesInput, "res->matchesInput");
     }
 
     /* Value creators. */