Bug 711266 - JSRope::flatten needs unconditional post barriers; r=billm
authorTerrence Cole <terrence@mozilla.com>
Thu, 15 Dec 2011 17:34:59 -0800
changeset 82739 aa1c4b29d4e4eda6bcc811c69c4cbd8a3258e448
parent 82738 ec45fad57ec61bcc7b3b28b2cc82d6e7c40fca48
child 82740 b9bd2420e1116ef682d0cdc9de28bb7d8a39be7b
push id4092
push usertcole@mozilla.com
push dateFri, 16 Dec 2011 01:39:17 +0000
treeherdermozilla-inbound@aa1c4b29d4e4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs711266
milestone11.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 711266 - JSRope::flatten needs unconditional post barriers; r=billm For incremental barriers we check if compartment()->needsBarrier to defer the cost when we are not in a GC cycle. We cannot do this for cross-generation barriers. We need to remove the checks on the post barriers and rename the enum to something more specific.
js/src/vm/String-inl.h
js/src/vm/String.cpp
js/src/vm/String.h
--- a/js/src/vm/String-inl.h
+++ b/js/src/vm/String-inl.h
@@ -88,16 +88,18 @@ JSString::validateLength(JSContext *cx, 
 }
 
 JS_ALWAYS_INLINE void
 JSRope::init(JSString *left, JSString *right, size_t length)
 {
     d.lengthAndFlags = buildLengthAndFlags(length, ROPE_BIT);
     d.u1.left = left;
     d.s.u2.right = right;
+    JSString::writeBarrierPost(d.u1.left, &d.u1.left);
+    JSString::writeBarrierPost(d.s.u2.right, &d.s.u2.right);
 }
 
 JS_ALWAYS_INLINE JSRope *
 JSRope::new_(JSContext *cx, JSString *left, JSString *right, size_t length)
 {
     if (!validateLength(cx, length))
         return NULL;
     JSRope *str = (JSRope *)js_NewGCString(cx);
@@ -108,16 +110,17 @@ JSRope::new_(JSContext *cx, JSString *le
 }
 
 JS_ALWAYS_INLINE void
 JSDependentString::init(JSLinearString *base, const jschar *chars, size_t length)
 {
     d.lengthAndFlags = buildLengthAndFlags(length, DEPENDENT_BIT);
     d.u1.chars = chars;
     d.s.u2.base = base;
+    JSString::writeBarrierPost(d.s.u2.base, &d.s.u2.base);
 }
 
 JS_ALWAYS_INLINE JSDependentString *
 JSDependentString::new_(JSContext *cx, JSLinearString *base, const jschar *chars, size_t length)
 {
     /* Try to avoid long chains of dependent strings. */
     while (base->isDependent())
         base = base->asDependent().base();
--- a/js/src/vm/String.cpp
+++ b/js/src/vm/String.cpp
@@ -182,39 +182,38 @@ JSRope::flattenInternal(JSContext *maybe
     jschar *wholeChars;
     JSString *str = this;
     jschar *pos;
 
     if (this->leftChild()->isExtensible()) {
         JSExtensibleString &left = this->leftChild()->asExtensible();
         size_t capacity = left.capacity();
         if (capacity >= wholeLength) {
-            if (b == WithBarrier) {
+            if (b == WithIncrementalBarrier) {
                 JSString::writeBarrierPre(d.u1.left);
                 JSString::writeBarrierPre(d.s.u2.right);
             }
 
             wholeCapacity = capacity;
             wholeChars = const_cast<jschar *>(left.chars());
             size_t bits = left.d.lengthAndFlags;
             pos = wholeChars + (bits >> LENGTH_SHIFT);
             left.d.lengthAndFlags = bits ^ (EXTENSIBLE_FLAGS | DEPENDENT_BIT);
             left.d.s.u2.base = (JSLinearString *)this;  /* will be true on exit */
-            if (b == WithBarrier)
-                JSString::writeBarrierPost(this, &left.d.s.u2.base);
+            JSString::writeBarrierPost(left.d.s.u2.base, &left.d.s.u2.base);
             goto visit_right_child;
         }
     }
 
     if (!AllocChars(maybecx, wholeLength, &wholeChars, &wholeCapacity))
         return NULL;
 
     pos = wholeChars;
     first_visit_node: {
-        if (b == WithBarrier) {
+        if (b == WithIncrementalBarrier) {
             JSString::writeBarrierPre(str->d.u1.left);
             JSString::writeBarrierPre(str->d.s.u2.right);
         }
 
         JSString &left = *str->d.u1.left;
         str->d.u1.chars = pos;
         if (left.isRope()) {
             left.d.s.u3.parent = str;          /* Return to this when 'left' done, */
@@ -245,32 +244,31 @@ JSRope::flattenInternal(JSContext *maybe
             str->d.lengthAndFlags = buildLengthAndFlags(wholeLength, EXTENSIBLE_FLAGS);
             str->d.u1.chars = wholeChars;
             str->d.s.u2.capacity = wholeCapacity;
             return &this->asFlat();
         }
         size_t progress = str->d.lengthAndFlags;
         str->d.lengthAndFlags = buildLengthAndFlags(pos - str->d.u1.chars, DEPENDENT_BIT);
         str->d.s.u2.base = (JSLinearString *)this;       /* will be true on exit */
-        if (b == WithBarrier)
-            JSString::writeBarrierPost(this, &str->d.s.u2.base);
+        JSString::writeBarrierPost(str->d.s.u2.base, &str->d.s.u2.base);
         str = str->d.s.u3.parent;
         if (progress == 0x200)
             goto visit_right_child;
         JS_ASSERT(progress == 0x300);
         goto finish_node;
     }
 }
 
 JSFlatString *
 JSRope::flatten(JSContext *maybecx)
 {
 #if JSGC_INCREMENTAL
     if (compartment()->needsBarrier())
-        return flattenInternal<WithBarrier>(maybecx);
+        return flattenInternal<WithIncrementalBarrier>(maybecx);
     else
         return flattenInternal<NoBarrier>(maybecx);
 #else
     return flattenInternal<NoBarrier>(maybecx);
 #endif
 }
 
 JSString * JS_FASTCALL
--- a/js/src/vm/String.h
+++ b/js/src/vm/String.h
@@ -421,17 +421,17 @@ class JSString : public js::gc::Cell
 
     static inline void writeBarrierPre(JSString *str);
     static inline void writeBarrierPost(JSString *str, void *addr);
     static inline bool needWriteBarrierPre(JSCompartment *comp);
 };
 
 class JSRope : public JSString
 {
-    enum UsingBarrier { WithBarrier, NoBarrier };
+    enum UsingBarrier { WithIncrementalBarrier, NoBarrier };
     template<UsingBarrier b>
     JSFlatString *flattenInternal(JSContext *cx);
 
     friend class JSString;
     JSFlatString *flatten(JSContext *cx);
 
     void init(JSString *left, JSString *right, size_t length);