Bug 1018311 part 1 - Refactor JSDependentString a bit. r=luke
authorJan de Mooij <jdemooij@mozilla.com>
Tue, 03 Jun 2014 13:12:46 +0200
changeset 205556 bb9c83bfcee0ce57911841ee1edf5be97d450e9c
parent 205555 b200395fd247c5f872b9a49e3ae70e6a1b810db6
child 205557 303e9291298bab01e3e76a4f42dff38a3d861eeb
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1018311
milestone32.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 1018311 part 1 - Refactor JSDependentString a bit. r=luke
js/src/jsstr.cpp
js/src/vm/String-inl.h
js/src/vm/String.h
--- a/js/src/jsstr.cpp
+++ b/js/src/jsstr.cpp
@@ -4077,22 +4077,24 @@ js_NewDependentString(JSContext *cx, JSS
 
     JSLinearString *base = baseArg->ensureLinear(cx);
     if (!base)
         return nullptr;
 
     if (start == 0 && length == base->length())
         return base;
 
-    const jschar *chars = base->chars() + start;
-
-    if (JSLinearString *staticStr = cx->staticStrings().lookup(chars, length))
-        return staticStr;
-
-    return JSDependentString::new_(cx, base, chars, length);
+    {
+        AutoCheckCannotGC nogc;
+        const jschar *chars = base->twoByteChars(nogc) + start;
+        if (JSLinearString *staticStr = cx->staticStrings().lookup(chars, length))
+            return staticStr;
+    }
+
+    return JSDependentString::new_(cx, base, start, length);
 }
 
 template <AllowGC allowGC>
 JSFlatString *
 js_NewStringCopyN(ExclusiveContext *cx, const jschar *s, size_t n)
 {
     if (JSFatInlineString::twoByteLengthFits(n))
         return NewFatInlineString<allowGC>(cx, TwoByteChars(s, n));
--- a/js/src/vm/String-inl.h
+++ b/js/src/vm/String-inl.h
@@ -16,68 +16,66 @@
 #include "gc/Marking.h"
 
 #include "jsgcinlines.h"
 
 namespace js {
 
 template <AllowGC allowGC>
 static MOZ_ALWAYS_INLINE JSInlineString *
+AllocateFatInlineString(ThreadSafeContext *cx, size_t len, jschar **chars)
+{
+    MOZ_ASSERT(JSFatInlineString::twoByteLengthFits(len));
+
+    if (JSInlineString::twoByteLengthFits(len)) {
+        JSInlineString *str = JSInlineString::new_<allowGC>(cx);
+        if (!str)
+            return nullptr;
+        *chars = str->initTwoByte(len);
+        return str;
+    }
+
+    JSFatInlineString *str = JSFatInlineString::new_<allowGC>(cx);
+    if (!str)
+        return nullptr;
+    *chars = str->initTwoByte(len);
+    return str;
+}
+
+template <AllowGC allowGC>
+static MOZ_ALWAYS_INLINE JSInlineString *
 NewFatInlineString(ThreadSafeContext *cx, JS::Latin1Chars chars)
 {
     size_t len = chars.length();
-    JS_ASSERT(JSFatInlineString::twoByteLengthFits(len));
 
-    JSInlineString *str;
     jschar *p;
-    if (JSInlineString::twoByteLengthFits(len)) {
-        str = JSInlineString::new_<allowGC>(cx);
-        if (!str)
-            return nullptr;
-        p = str->initTwoByte(len);
-    } else {
-        JSFatInlineString *fatstr = JSFatInlineString::new_<allowGC>(cx);
-        if (!fatstr)
-            return nullptr;
-        p = fatstr->initTwoByte(len);
-        str = fatstr;
-    }
+    JSInlineString *str = AllocateFatInlineString<allowGC>(cx, len, &p);
+    if (!str)
+        return nullptr;
 
     for (size_t i = 0; i < len; ++i)
         p[i] = static_cast<jschar>(chars[i]);
     p[len] = '\0';
     return str;
 }
 
 template <AllowGC allowGC>
 static MOZ_ALWAYS_INLINE JSInlineString *
 NewFatInlineString(ExclusiveContext *cx, JS::TwoByteChars chars)
 {
-    size_t len = chars.length();
-
     /*
      * Don't bother trying to find a static atom; measurement shows that not
      * many get here (for one, Atomize is catching them).
      */
-    JS_ASSERT(JSFatInlineString::twoByteLengthFits(len));
 
-    JSInlineString *str;
+    size_t len = chars.length();
     jschar *storage;
-    if (JSInlineString::twoByteLengthFits(len)) {
-        str = JSInlineString::new_<allowGC>(cx);
-        if (!str)
-            return nullptr;
-        storage = str->initTwoByte(len);
-    } else {
-        JSFatInlineString *fatstr = JSFatInlineString::new_<allowGC>(cx);
-        if (!fatstr)
-            return nullptr;
-        storage = fatstr->initTwoByte(len);
-        str = fatstr;
-    }
+    JSInlineString *str = AllocateFatInlineString<allowGC>(cx, len, &storage);
+    if (!str)
+        return nullptr;
 
     mozilla::PodCopy(storage, chars.start().get(), len);
     storage[len] = 0;
     return str;
 }
 
 static inline void
 StringWriteBarrierPost(js::ThreadSafeContext *maybecx, JSString **strp)
@@ -134,71 +132,72 @@ JSRope::new_(js::ThreadSafeContext *cx,
 inline void
 JSRope::markChildren(JSTracer *trc)
 {
     js::gc::MarkStringUnbarriered(trc, &d.s.u2.left, "left child");
     js::gc::MarkStringUnbarriered(trc, &d.s.u3.right, "right child");
 }
 
 MOZ_ALWAYS_INLINE void
-JSDependentString::init(js::ThreadSafeContext *cx, JSLinearString *base, const jschar *chars,
+JSDependentString::init(js::ThreadSafeContext *cx, JSLinearString *base, size_t start,
                         size_t length)
 {
-    JS_ASSERT(!js::IsPoisonedPtr(base));
+    MOZ_ASSERT(!js::IsPoisonedPtr(base));
+    MOZ_ASSERT(start + length <= base->length());
     d.u1.length = length;
     d.u1.flags = DEPENDENT_FLAGS;
-    d.s.u2.nonInlineCharsTwoByte = chars;
+    JS::AutoCheckCannotGC nogc;
+    d.s.u2.nonInlineCharsTwoByte = base->twoByteChars(nogc) + start;
     d.s.u3.base = base;
     js::StringWriteBarrierPost(cx, reinterpret_cast<JSString **>(&d.s.u3.base));
 }
 
 MOZ_ALWAYS_INLINE JSLinearString *
-JSDependentString::new_(js::ExclusiveContext *cx,
-                        JSLinearString *baseArg, const jschar *chars, size_t length)
+JSDependentString::new_(js::ExclusiveContext *cx, JSLinearString *baseArg, size_t start,
+                        size_t length)
 {
     /* Try to avoid long chains of dependent strings. */
-    while (baseArg->isDependent())
+    while (baseArg->isDependent()) {
+        start += baseArg->asDependent().baseOffset();
         baseArg = baseArg->asDependent().base();
-
-    JS_ASSERT(baseArg->isFlat());
+    }
 
-    /*
-     * The chars we are pointing into must be owned by something in the chain
-     * of dependent or undepended strings kept alive by our base pointer.
-     */
-#ifdef DEBUG
-    for (JSLinearString *b = baseArg; ; b = b->base()) {
-        if (chars >= b->chars() && chars < b->chars() + b->length() &&
-            length <= b->length() - (chars - b->chars()))
-        {
-            break;
-        }
-    }
-#endif
+    MOZ_ASSERT(start + length <= baseArg->length());
+    MOZ_ASSERT(baseArg->isFlat());
 
     /*
      * Do not create a string dependent on inline chars from another string,
      * both to avoid the awkward moving-GC hazard this introduces and because it
      * is more efficient to immediately undepend here.
      */
-    if (JSFatInlineString::twoByteLengthFits(length))
-        return js::NewFatInlineString<js::CanGC>(cx, JS::TwoByteChars(chars, length));
+    if (JSFatInlineString::twoByteLengthFits(length)) {
+        JS::Rooted<JSLinearString*> base(cx, baseArg);
+        jschar *chars;
+        JSInlineString *s = js::AllocateFatInlineString<js::CanGC>(cx, length, &chars);
+        if (!s)
+            return nullptr;
+
+        JS::AutoCheckCannotGC nogc;
+        mozilla::PodCopy(chars, base->twoByteChars(nogc) + start, length);
+        chars[length] = 0;
+        return s;
+    }
 
     JSDependentString *str = (JSDependentString *)js_NewGCString<js::NoGC>(cx);
     if (str) {
-        str->init(cx, baseArg, chars, length);
+        str->init(cx, baseArg, start, length);
         return str;
     }
 
     JS::Rooted<JSLinearString*> base(cx, baseArg);
 
     str = (JSDependentString *)js_NewGCString<js::CanGC>(cx);
     if (!str)
         return nullptr;
-    str->init(cx, base, chars, length);
+    str->init(cx, base, start, length);
     return str;
 }
 
 inline void
 JSString::markBase(JSTracer *trc)
 {
     JS_ASSERT(hasBase());
     js::gc::MarkStringUnbarriered(trc, &d.s.u3.base, "base");
--- a/js/src/vm/String.h
+++ b/js/src/vm/String.h
@@ -654,29 +654,38 @@ JS_STATIC_ASSERT(sizeof(JSLinearString) 
 
 class JSDependentString : public JSLinearString
 {
     bool copyNonPureCharsZ(js::ThreadSafeContext *cx, js::ScopedJSFreePtr<jschar> &out) const;
 
     friend class JSString;
     JSFlatString *undepend(js::ExclusiveContext *cx);
 
-    void init(js::ThreadSafeContext *cx, JSLinearString *base, const jschar *chars,
+    void init(js::ThreadSafeContext *cx, JSLinearString *base, size_t start,
               size_t length);
 
     /* Vacuous and therefore unimplemented. */
     bool isDependent() const MOZ_DELETE;
     JSDependentString &asDependent() const MOZ_DELETE;
 
     /* Hide chars(), nonInlineChars() is more efficient. */
     const jschar *chars() const MOZ_DELETE;
 
+    /* The offset of this string's chars in base->chars(). */
+    size_t baseOffset() const {
+        MOZ_ASSERT(JSString::isDependent());
+        JS::AutoCheckCannotGC nogc;
+        size_t offset = twoByteChars(nogc) - base()->twoByteChars(nogc);
+        MOZ_ASSERT(offset < base()->length());
+        return offset;
+    }
+
   public:
     static inline JSLinearString *new_(js::ExclusiveContext *cx, JSLinearString *base,
-                                       const jschar *chars, size_t length);
+                                       size_t start, size_t length);
 };
 
 JS_STATIC_ASSERT(sizeof(JSDependentString) == sizeof(JSString));
 
 class JSFlatString : public JSLinearString
 {
     /* Vacuous and therefore unimplemented. */
     JSFlatString *ensureFlat(JSContext *cx) MOZ_DELETE;