Bug 784808 - Remove isInline GC size class check; r=luke
authorTerrence Cole <terrence@mozilla.com>
Wed, 22 Aug 2012 12:19:07 -0700
changeset 105716 539643ca53ec94b6d520c5ee5bc9ccfa63dbec7b
parent 105715 ca53a84f1a9439b66a8c631e389ca1f4684e7035
child 105717 b22fb91ad7e207e91bfd63ecf18410d17ee356b3
push id55
push usershu@rfrn.org
push dateThu, 30 Aug 2012 01:33:09 +0000
reviewersluke
bugs784808
milestone18.0a1
Bug 784808 - Remove isInline GC size class check; r=luke We depend on a size class check because JSShortStrings can have their chars() point to the middle of its inline buffer for a slight speedup in Int32ToString and IndexToString. Getting rid of the size class check allows us to inline the isInline check. This work paves the way for a fast ensureNotInline and removal of getAllocKind as a requirement for string placement in arenas.
js/src/jsnum.cpp
js/src/vm/String-inl.h
js/src/vm/String.cpp
js/src/vm/String.h
--- a/js/src/jsnum.cpp
+++ b/js/src/jsnum.cpp
@@ -533,27 +533,26 @@ js::Int32ToString(JSContext *cx, int32_t
     JSCompartment *c = cx->compartment;
     if (JSFixedString *str = c->dtoaCache.lookup(10, si))
         return str;
 
     JSShortString *str = js_NewGCShortString(cx);
     if (!str)
         return NULL;
 
-    jschar *storage = str->inlineStorageBeforeInit();
-    RangedPtr<jschar> end(storage + JSShortString::MAX_SHORT_LENGTH,
-                          storage, JSShortString::MAX_SHORT_LENGTH + 1);
+    jschar buffer[JSShortString::MAX_SHORT_LENGTH + 1];
+    RangedPtr<jschar> end(buffer + JSShortString::MAX_SHORT_LENGTH,
+                          buffer, JSShortString::MAX_SHORT_LENGTH + 1);
     *end = '\0';
-
     RangedPtr<jschar> start = BackfillIndexInCharBuffer(ui, end);
-
     if (si < 0)
         *--start = '-';
 
-    str->initAtOffsetInBuffer(start.get(), end - start);
+    jschar *dst = str->init(end - start);
+    PodCopy(dst, start.get(), end - start + 1);
 
     c->dtoaCache.cache(10, si, str);
     return str;
 }
 
 /* Returns a non-NULL pointer to inside cbuf.  */
 static char *
 IntToCString(ToCStringBuf *cbuf, int i, int base = 10)
@@ -1298,24 +1297,24 @@ IndexToString(JSContext *cx, uint32_t in
     JSCompartment *c = cx->compartment;
     if (JSFixedString *str = c->dtoaCache.lookup(10, index))
         return str;
 
     JSShortString *str = js_NewGCShortString(cx);
     if (!str)
         return NULL;
 
-    jschar *storage = str->inlineStorageBeforeInit();
-    size_t length = JSShortString::MAX_SHORT_LENGTH;
-    const RangedPtr<jschar> end(storage + length, storage, length + 1);
+    jschar buffer[JSShortString::MAX_SHORT_LENGTH + 1];
+    RangedPtr<jschar> end(buffer + JSShortString::MAX_SHORT_LENGTH,
+                          buffer, JSShortString::MAX_SHORT_LENGTH + 1);
     *end = '\0';
-
     RangedPtr<jschar> start = BackfillIndexInCharBuffer(index, end);
 
-    str->initAtOffsetInBuffer(start.get(), end - start);
+    jschar *dst = str->init(end - start);
+    PodCopy(dst, start.get(), end - start + 1);
 
     c->dtoaCache.cache(10, index, str);
     return str;
 }
 
 bool JS_FASTCALL
 NumberValueToStringBuffer(JSContext *cx, const Value &v, StringBuffer &sb)
 {
--- a/js/src/vm/String-inl.h
+++ b/js/src/vm/String-inl.h
@@ -263,25 +263,16 @@ JSInlineString::resetLength(size_t lengt
 
 JS_ALWAYS_INLINE JSShortString *
 JSShortString::new_(JSContext *cx)
 {
     return js_NewGCShortString(cx);
 }
 
 JS_ALWAYS_INLINE void
-JSShortString::initAtOffsetInBuffer(const jschar *chars, size_t length)
-{
-    JS_ASSERT(lengthFits(length + (chars - d.inlineStorage)));
-    JS_ASSERT(chars >= d.inlineStorage && chars < d.inlineStorage + MAX_SHORT_LENGTH);
-    d.lengthAndFlags = buildLengthAndFlags(length, FIXED_FLAGS);
-    d.u1.chars = chars;
-}
-
-JS_ALWAYS_INLINE void
 JSExternalString::init(const jschar *chars, size_t length, const JSStringFinalizer *fin)
 {
     JS_ASSERT(fin);
     JS_ASSERT(fin->finalize);
     d.lengthAndFlags = buildLengthAndFlags(length, FIXED_FLAGS);
     d.u1.chars = chars;
     d.s.u2.externalFinalizer = fin;
 }
@@ -425,38 +416,37 @@ JSString::finalize(js::FreeOp *fop)
         JS_ASSERT(isDependent() || isRope());
 }
 
 inline void
 JSFlatString::finalize(js::FreeOp *fop)
 {
     JS_ASSERT(!isShort());
 
-    /*
-     * This check depends on the fact that 'chars' is only initialized to the
-     * beginning of inlineStorage. E.g., this is not the case for short strings.
-     */
     if (chars() != d.inlineStorage)
         fop->free_(const_cast<jschar *>(chars()));
 }
 
 inline void
 JSShortString::finalize(js::FreeOp *fop)
 {
-    JS_ASSERT(JSString::isShort());
+    JS_ASSERT(isShort());
+
+    if (chars() != d.inlineStorage)
+        fop->free_(const_cast<jschar *>(chars()));
 }
 
 inline void
 JSAtom::finalize(js::FreeOp *fop)
 {
     JS_ASSERT(JSString::isAtom());
-    if (getAllocKind() == js::gc::FINALIZE_STRING)
-        JSFlatString::finalize(fop);
-    else
-        JS_ASSERT(getAllocKind() == js::gc::FINALIZE_SHORT_STRING);
+    JS_ASSERT(JSString::isFlat());
+
+    if (chars() != d.inlineStorage)
+        fop->free_(const_cast<jschar *>(chars()));
 }
 
 inline void
 JSExternalString::finalize(js::FreeOp *fop)
 {
     const JSStringFinalizer *fin = externalFinalizer();
     fin->finalize(fin, const_cast<jschar *>(chars()));
 }
--- a/js/src/vm/String.cpp
+++ b/js/src/vm/String.cpp
@@ -12,35 +12,25 @@
 #include "String.h"
 #include "String-inl.h"
 
 #include "jsobjinlines.h"
 
 using namespace mozilla;
 using namespace js;
 
+#ifdef DEBUG
 bool
 JSString::isShort() const
 {
     bool is_short = (getAllocKind() == gc::FINALIZE_SHORT_STRING);
     JS_ASSERT_IF(is_short, isFixed());
     return is_short;
 }
-
-bool
-JSString::isFixed() const
-{
-    return isFlat() && !isExtensible();
-}
-
-bool
-JSString::isInline() const
-{
-    return isFixed() && (d.u1.chars == d.inlineStorage || isShort());
-}
+#endif
 
 bool
 JSString::isExternal() const
 {
     bool is_external = (getAllocKind() == gc::FINALIZE_EXTERNAL_STRING);
     JS_ASSERT_IF(is_external, isFixed());
     return is_external;
 }
--- a/js/src/vm/String.h
+++ b/js/src/vm/String.h
@@ -181,17 +181,17 @@ class JSString : public js::gc::Cell
      *   Rope         0000       0000
      *   Linear       -         !0000
      *   HasBase      -          xxx1
      *   Dependent    0001       0001
      *   Flat         -          isLinear && !isDependent
      *   Undepended   0011       0011
      *   Extensible   0010       0010
      *   Fixed        0100       isFlat && !isExtensible
-     *   Inline       0100       isFixed && (u1.chars == inlineStorage || isShort || isInt32)
+     *   Inline       0100       isFixed && (u1.chars == inlineStorage) || isInt32)
      *   Short        0100       header in FINALIZE_SHORT_STRING arena
      *   External     0100       header in FINALIZE_EXTERNAL_STRING arena
      *   Int32        0110       x110 (NYI, Bug 654190)
      *   Atom         1000       1xxx
      *   InlineAtom   1000       1000 && is Inline
      *   ShortAtom    1000       1000 && is Short
      *   Int32Atom    1110       1110 (NYI, Bug 654190)
      *
@@ -325,27 +325,33 @@ class JSString : public js::gc::Cell
     }
 
     JS_ALWAYS_INLINE
     JSExtensibleString &asExtensible() const {
         JS_ASSERT(isExtensible());
         return *(JSExtensibleString *)this;
     }
 
-    /* For hot code, prefer other type queries. */
-    bool isShort() const;
-    bool isFixed() const;
-    bool isInline() const;
+    JS_ALWAYS_INLINE
+    bool isFixed() const {
+        return isFlat() && !isExtensible();
+    }
 
     JS_ALWAYS_INLINE
     JSFixedString &asFixed() const {
         JS_ASSERT(isFixed());
         return *(JSFixedString *)this;
     }
 
+    JS_ALWAYS_INLINE
+    bool isInline() const {
+        return isFixed() && (d.u1.chars == d.inlineStorage);
+    }
+
+    /* For hot code, prefer other type queries. */
     bool isExternal() const;
 
     JS_ALWAYS_INLINE
     JSExternalString &asExternal() const {
         JS_ASSERT(isExternal());
         return *(JSExternalString *)this;
     }
 
@@ -397,16 +403,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);
     static inline void readBarrier(JSString *str);
 
     static inline js::ThingRootKind rootKind() { return js::THING_ROOT_STRING; }
 
 #ifdef DEBUG
+    bool isShort() const;
     void dump();
     bool equals(const char *s);
 #endif
 
   private:
     JSString() MOZ_DELETE;
     JSString(const JSString &other) MOZ_DELETE;
     void operator=(const JSString &other) MOZ_DELETE;
@@ -588,31 +595,25 @@ class JSShortString : public JSInlineStr
     }
 
   protected: /* to fool clang into not warning this is unused */
     jschar inlineStorageExtension[INLINE_EXTENSION_CHARS];
 
   public:
     static inline JSShortString *new_(JSContext *cx);
 
-    jschar *inlineStorageBeforeInit() {
-        return d.inlineStorage;
-    }
-
-    inline void initAtOffsetInBuffer(const jschar *chars, size_t length);
-
     static const size_t MAX_SHORT_LENGTH = JSString::NUM_INLINE_CHARS +
                                            INLINE_EXTENSION_CHARS
                                            -1 /* null terminator */;
 
     static bool lengthFits(size_t length) {
         return length <= MAX_SHORT_LENGTH;
     }
 
-    /* Only called by the GC for strings with the FINALIZE_EXTERNAL_STRING kind. */
+    /* Only called by the GC for strings with the FINALIZE_SHORT_STRING kind. */
 
     JS_ALWAYS_INLINE void finalize(js::FreeOp *fop);
 };
 
 JS_STATIC_ASSERT(sizeof(JSShortString) == 2 * sizeof(JSString));
 
 class JSExternalString : public JSFixedString
 {