From 1913b99eb03fdd5b04ab963c8c7916bddbd869df Mon Sep 17 00:00:00 2001 From: Luke Wagner Date: Thu, 31 Mar 2011 16:07:21 -0700 Subject: [PATCH] Bug 639883 - Use JSString (not JSShortString) for inline really short inline strings (r=njn) --HG-- extra : rebase_source : 5d18f67f841864064e032836014978cf51b52f5c --- js/src/jsstr.cpp | 20 +++-- js/src/jsstr.h | 169 +++++++++++++++++++++++------------------- js/src/jsstrinlines.h | 45 ++++++++++- 3 files changed, 153 insertions(+), 81 deletions(-) diff --git a/js/src/jsstr.cpp b/js/src/jsstr.cpp index e91ba270283..6be6afe6b5f 100644 --- a/js/src/jsstr.cpp +++ b/js/src/jsstr.cpp @@ -3575,28 +3575,38 @@ js_NewString(JSContext *cx, jschar *chars, size_t length) return JSFixedString::new_(cx, chars, length); } -static JS_ALWAYS_INLINE JSShortString * +static JS_ALWAYS_INLINE JSFixedString * NewShortString(JSContext *cx, const jschar *chars, size_t 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(JSShortString::lengthFits(length)); - JSShortString *str = js_NewGCShortString(cx); + JSInlineString *str = JSInlineString::lengthFits(length) + ? JSInlineString::new_(cx) + : JSShortString::new_(cx); if (!str) return NULL; + jschar *storage = str->init(length); PodCopy(storage, chars, length); storage[length] = 0; return str; } -static JSShortString * +static JSInlineString * NewShortString(JSContext *cx, const char *chars, size_t length) { JS_ASSERT(JSShortString::lengthFits(length)); - JSShortString *str = js_NewGCShortString(cx); + JSInlineString *str = JSInlineString::lengthFits(length) + ? JSInlineString::new_(cx) + : JSShortString::new_(cx); if (!str) return NULL; - jschar *storage = str->init(length); + jschar *storage = str->init(length); if (js_CStringsAreUTF8) { #ifdef DEBUG size_t oldLength = length; diff --git a/js/src/jsstr.h b/js/src/jsstr.h index dc62aa8d8b3..33aac3125ab 100644 --- a/js/src/jsstr.h +++ b/js/src/jsstr.h @@ -77,8 +77,8 @@ * property via a flat string's "extensible" property. * * - To avoid allocating small char arrays, short strings can be stored inline - * in the string header. To increase the max size of such inline strings, - * double-wide string headers (JSShortString) can be used. + * in the string header (JSInlineString). To increase the max size of such + * inline strings, extra-large string headers can be used (JSShortString). * * - To avoid comparing O(n) string equality comparison, strings can be * canonicalized to "atoms" (JSAtom) such that there is a single atom with a @@ -100,27 +100,31 @@ * C++ type operations+fields / invariants+properties * * JSString (abstract) getCharsZ, getChars, length / - - * | \ - * | JSRope leftChild, rightChild / - - * | + * | \ + * | JSRope leftChild, rightChild / - + * | * JSLinearString (abstract) chars / not null-terminated - * | \ - * | JSDependentString base / - - * | + * | \ + * | JSDependentString base / - + * | * JSFlatString (abstract) chars / not null-terminated - * | \ - * | JSExtensibleString capacity / no external pointers into char array - * | + * | \ + * | JSExtensibleString capacity / no external pointers into char array + * | * JSFixedString - / may have external pointers into char array - * | \ \ - * | \ JSExternalString - / chars stored in header - * | \ - * | JSShortString - / chars stored in header - * | | - * JSAtom | - / string equality === pointer equality - * | \ | - * | JSShortAtom - / atomized JSShortString - * | + * | \ \ + * | \ JSExternalString - / char array memory managed by embedding + * | \ + * | JSInlineString - / chars stored in header + * | | \ + * | | JSShortString - / header is fat + * | | | + * JSAtom | | - / string equality === pointer equality + * | \ | | + * | JSInlineAtom | - / atomized JSInlineString + * | \ | + * | JSShortAtom - / atomized JSShortString + * | * JSStaticAtom - / header and chars statically allocated * * Classes marked with (abstract) above are not literally C++ Abstract Base @@ -149,7 +153,7 @@ class JSString : public js::gc::Cell JSString *left; /* JSRope */ } u1; union { - jschar inlineStorage[NUM_INLINE_CHARS]; /* JSShortString */ + jschar inlineStorage[NUM_INLINE_CHARS]; /* JS(Inline|Short)String */ struct { union { JSLinearString *base; /* JSDependentString */ @@ -187,6 +191,7 @@ class JSString : public js::gc::Cell * JSFlatString xx00 * JSExtensibleString 1100 * JSFixedString xy00 where xy != 11 + * JSInlineString 0100 and chars == inlineStorage * JSShortString 0100 and in FINALIZE_SHORT_STRING arena * JSExternalString 0100 and in FINALIZE_EXTERNAL_STRING arena * JSAtom x000 @@ -480,6 +485,63 @@ class JSFixedString : public JSFlatString JS_STATIC_ASSERT(sizeof(JSFixedString) == sizeof(JSString)); +class JSInlineString : public JSFixedString +{ + static const size_t MAX_INLINE_LENGTH = NUM_INLINE_CHARS - 1; + + public: + static inline JSInlineString *new_(JSContext *cx); + + inline jschar *init(size_t length); + + inline void resetLength(size_t length); + + static bool lengthFits(size_t length) { + return length <= MAX_INLINE_LENGTH; + } + +}; + +JS_STATIC_ASSERT(sizeof(JSInlineString) == sizeof(JSString)); + +class JSShortString : public JSInlineString +{ + /* This can be any value that is a multiple of sizeof(gc::FreeCell). */ + static const size_t INLINE_EXTENSION_CHARS = sizeof(JSString::Data) / sizeof(jschar); + + static void staticAsserts() { + JS_STATIC_ASSERT(INLINE_EXTENSION_CHARS % sizeof(js::gc::FreeCell) == 0); + JS_STATIC_ASSERT(MAX_SHORT_LENGTH + 1 == + (sizeof(JSShortString) - + offsetof(JSShortString, d.inlineStorage)) / sizeof(jschar)); + } + + 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. */ + + JS_ALWAYS_INLINE void finalize(JSContext *cx); +}; + +JS_STATIC_ASSERT(sizeof(JSShortString) == 2 * sizeof(JSString)); + class JSExternalString : public JSFixedString { static void staticAsserts() { @@ -520,59 +582,6 @@ class JSExternalString : public JSFixedString JS_STATIC_ASSERT(sizeof(JSExternalString) == sizeof(JSString)); -class JSShortString : public JSFixedString -{ - /* This can be any value that is a multiple of sizeof(gc::FreeCell). */ - static const size_t INLINE_EXTENSION_CHARS = sizeof(JSString::Data) / sizeof(jschar); - - static void staticAsserts() { - JS_STATIC_ASSERT(INLINE_EXTENSION_CHARS % sizeof(js::gc::FreeCell) == 0); - JS_STATIC_ASSERT(MAX_SHORT_LENGTH + 1 == - (sizeof(JSShortString) - - offsetof(JSShortString, d.inlineStorage)) / sizeof(jschar)); - } - - jschar inlineStorageExtension[INLINE_EXTENSION_CHARS]; - - public: - jschar *inlineStorageBeforeInit() { - return d.inlineStorage; - } - - jschar *init(size_t length) { - JS_ASSERT(lengthFits(length)); - d.u1.chars = d.inlineStorage; - d.lengthAndFlags = buildLengthAndFlags(length, FIXED_FLAGS); - return d.inlineStorage; - } - - void resetLength(size_t length) { - JS_ASSERT(lengthFits(length)); - d.lengthAndFlags = buildLengthAndFlags(length, FIXED_FLAGS); - } - - void 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; - } - - static const size_t MAX_SHORT_LENGTH = JSString::NUM_INLINE_CHARS + - INLINE_EXTENSION_CHARS - -1 /* null terminator */; - - static inline bool lengthFits(size_t length) { - return length <= MAX_SHORT_LENGTH; - } - - /* Only called by the GC for strings with the FINALIZE_EXTERNAL_STRING kind. */ - - JS_ALWAYS_INLINE void finalize(JSContext *cx); -}; - -JS_STATIC_ASSERT(sizeof(JSShortString) == 2 * sizeof(JSString)); - class JSAtom : public JSFixedString { public: @@ -637,7 +646,17 @@ class JSAtom : public JSFixedString JS_STATIC_ASSERT(sizeof(JSAtom) == sizeof(JSString)); -class JSShortAtom : public JSShortString /*, JSAtom */ +class JSInlineAtom : public JSInlineString /*, JSAtom */ +{ + /* + * JSInlineAtom is not explicitly used and is only present for consistency. + * See Atomize() for how JSInlineStrings get morphed into JSInlineAtoms. + */ +}; + +JS_STATIC_ASSERT(sizeof(JSInlineAtom) == sizeof(JSInlineString)); + +class JSShortAtom : public JSShortString /*, JSInlineAtom */ { /* * JSShortAtom is not explicitly used and is only present for consistency. diff --git a/js/src/jsstrinlines.h b/js/src/jsstrinlines.h index 33d99946078..b4077720462 100644 --- a/js/src/jsstrinlines.h +++ b/js/src/jsstrinlines.h @@ -338,6 +338,43 @@ JSFixedString::morphInternedStringIntoAtom() return &asAtom(); } +JS_ALWAYS_INLINE JSInlineString * +JSInlineString::new_(JSContext *cx) +{ + return (JSInlineString *)js_NewGCString(cx); +} + +JS_ALWAYS_INLINE jschar * +JSInlineString::init(size_t length) +{ + d.lengthAndFlags = buildLengthAndFlags(length, FIXED_FLAGS); + d.u1.chars = d.inlineStorage; + JS_ASSERT(lengthFits(length) || (isShort() && JSShortString::lengthFits(length))); + return d.inlineStorage; +} + +JS_ALWAYS_INLINE void +JSInlineString::resetLength(size_t length) +{ + d.lengthAndFlags = buildLengthAndFlags(length, FIXED_FLAGS); + JS_ASSERT(lengthFits(length) || (isShort() && JSShortString::lengthFits(length))); +} + +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, intN type) { @@ -478,7 +515,13 @@ JSFlatString::finalize(JSRuntime *rt) { JS_ASSERT(!isShort()); rt->stringMemoryUsed -= length() * 2; - rt->free(const_cast(chars())); + + /* + * 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) + rt->free_(const_cast(chars())); } inline void