From 9a7af842af879dd4239c954eb9465ac85e09294d Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Mon, 25 Apr 2016 13:41:25 +0100 Subject: [PATCH] Bug 1259021 - Use in-place storage in AutoStableStringChars to avoid allocation for short strings r=jandem r=Waldo --- js/src/jsfriendapi.h | 24 ++++++++++++++++-------- js/src/vm/String.cpp | 43 ++++++++++++++++++++++++++----------------- 2 files changed, 42 insertions(+), 25 deletions(-) diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h index 3513731a6c9a..abc3906d2e48 100644 --- a/js/src/jsfriendapi.h +++ b/js/src/jsfriendapi.h @@ -9,6 +9,7 @@ #include "mozilla/Atomics.h" #include "mozilla/Casting.h" +#include "mozilla/Maybe.h" #include "mozilla/MemoryReporting.h" #include "mozilla/UniquePtr.h" @@ -1295,21 +1296,27 @@ GetErrorMessage(void* userRef, const unsigned errorNumber); */ class MOZ_STACK_CLASS AutoStableStringChars { + /* + * When copying string char, use this many bytes of inline storage. This is + * chosen to allow the inline string types to be copied without allocating. + * This is asserted in AutoStableStringChars::allocOwnChars. + */ + static const size_t InlineCapacity = 24; + /* Ensure the string is kept alive while we're using its chars. */ JS::RootedString s_; union { const char16_t* twoByteChars_; const JS::Latin1Char* latin1Chars_; }; + mozilla::Maybe> ownChars_; enum State { Uninitialized, Latin1, TwoByte }; State state_; - bool ownsChars_; public: explicit AutoStableStringChars(JSContext* cx) - : s_(cx), state_(Uninitialized), ownsChars_(false) + : s_(cx), state_(Uninitialized) {} - ~AutoStableStringChars(); MOZ_WARN_UNUSED_RESULT bool init(JSContext* cx, JSString* s); @@ -1335,16 +1342,16 @@ class MOZ_STACK_CLASS AutoStableStringChars mozilla::Range twoByteRange() const { MOZ_ASSERT(state_ == TwoByte); return mozilla::Range(twoByteChars_, - GetStringLength(s_)); + GetStringLength(s_)); } /* If we own the chars, transfer ownership to the caller. */ bool maybeGiveOwnershipToCaller() { MOZ_ASSERT(state_ != Uninitialized); - if (!ownsChars_) + if (!ownChars_.isSome() || !ownChars_->extractRawBuffer()) return false; state_ = Uninitialized; - ownsChars_ = false; + ownChars_.reset(); return true; } @@ -1353,8 +1360,9 @@ class MOZ_STACK_CLASS AutoStableStringChars void operator=(const AutoStableStringChars& other) = delete; bool baseIsInline(JS::Handle linearString); - bool copyLatin1Chars(JSContext*, JS::Handle linearString); - bool copyTwoByteChars(JSContext*, JS::Handle linearString); + template T* allocOwnChars(JSContext* cx, size_t count); + bool copyLatin1Chars(JSContext* cx, JS::Handle linearString); + bool copyTwoByteChars(JSContext* cx, JS::Handle linearString); bool copyAndInflateLatin1Chars(JSContext*, JS::Handle linearString); }; diff --git a/js/src/vm/String.cpp b/js/src/vm/String.cpp index ce9c9dfa721a..082f69da5959 100644 --- a/js/src/vm/String.cpp +++ b/js/src/vm/String.cpp @@ -874,17 +874,6 @@ StaticStrings::isStatic(JSAtom* atom) : isStatic(atom->twoByteChars(nogc), atom->length()); } -AutoStableStringChars::~AutoStableStringChars() -{ - if (ownsChars_) { - MOZ_ASSERT(state_ == Latin1 || state_ == TwoByte); - if (state_ == Latin1) - js_free(const_cast(latin1Chars_)); - else - js_free(const_cast(twoByteChars_)); - } -} - bool AutoStableStringChars::init(JSContext* cx, JSString* s) { @@ -944,10 +933,33 @@ bool AutoStableStringChars::baseIsInline(HandleLinearString linearString) return base->isInline(); } +template +T* +AutoStableStringChars::allocOwnChars(JSContext* cx, size_t count) +{ + static_assert( + InlineCapacity >= sizeof(JS::Latin1Char) * (JSFatInlineString::MAX_LENGTH_LATIN1 + 1) && + InlineCapacity >= sizeof(char16_t) * (JSFatInlineString::MAX_LENGTH_TWO_BYTE + 1), + "InlineCapacity too small to hold fat inline strings"); + + static_assert((JSString::MAX_LENGTH & mozilla::tl::MulOverflowMask::value) == 0, + "Size calculation can overflow"); + MOZ_ASSERT(count <= JSString::MAX_LENGTH); + size_t size = sizeof(T) * count; + + ownChars_.emplace(cx); + if (!ownChars_->resize(size)) { + ownChars_.reset(); + return nullptr; + } + + return reinterpret_cast(ownChars_->begin()); +} + bool AutoStableStringChars::copyAndInflateLatin1Chars(JSContext* cx, HandleLinearString linearString) { - char16_t* chars = cx->pod_malloc(linearString->length() + 1); + char16_t* chars = allocOwnChars(cx, linearString->length() + 1); if (!chars) return false; @@ -956,7 +968,6 @@ AutoStableStringChars::copyAndInflateLatin1Chars(JSContext* cx, HandleLinearStri chars[linearString->length()] = 0; state_ = TwoByte; - ownsChars_ = true; twoByteChars_ = chars; s_ = linearString; return true; @@ -966,7 +977,7 @@ bool AutoStableStringChars::copyLatin1Chars(JSContext* cx, HandleLinearString linearString) { size_t length = linearString->length(); - JS::Latin1Char* chars = cx->pod_malloc(length + 1); + JS::Latin1Char* chars = allocOwnChars(cx, length + 1); if (!chars) return false; @@ -974,7 +985,6 @@ AutoStableStringChars::copyLatin1Chars(JSContext* cx, HandleLinearString linearS chars[length] = 0; state_ = Latin1; - ownsChars_ = true; latin1Chars_ = chars; s_ = linearString; return true; @@ -984,7 +994,7 @@ bool AutoStableStringChars::copyTwoByteChars(JSContext* cx, HandleLinearString linearString) { size_t length = linearString->length(); - char16_t* chars = cx->pod_malloc(length + 1); + char16_t* chars = allocOwnChars(cx, length + 1); if (!chars) return false; @@ -992,7 +1002,6 @@ AutoStableStringChars::copyTwoByteChars(JSContext* cx, HandleLinearString linear chars[length] = 0; state_ = TwoByte; - ownsChars_ = true; twoByteChars_ = chars; s_ = linearString; return true;