Bug 676739 - Part 1: Suppress unnecessary temporary strings in js::ToAtom. r=luke
authorYaron Tausky <yaron.tausky@gmail.com>
Tue, 27 Aug 2013 11:17:41 +0200
changeset 159083 9cbd48387451e8cb14769eddecba4aedb41c594d
parent 159082 5c9f3fb14995931208c04d29ad4f6cc6616616ac
child 159084 3f272348bb63bcf92f615ec80d026968c16db50f
push id2961
push userlsblakk@mozilla.com
push dateMon, 28 Oct 2013 21:59:28 +0000
treeherdermozilla-beta@73ef4f13486f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs676739
milestone26.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 676739 - Part 1: Suppress unnecessary temporary strings in js::ToAtom. r=luke
js/src/jsatom.cpp
js/src/jsatom.h
js/src/jsnum.cpp
js/src/jsnum.h
--- a/js/src/jsatom.cpp
+++ b/js/src/jsatom.cpp
@@ -19,16 +19,17 @@
 #include "jsstr.h"
 #include "jstypes.h"
 
 #include "gc/Marking.h"
 #include "vm/Xdr.h"
 
 #include "jscntxtinlines.h"
 #include "jscompartmentinlines.h"
+#include "jsobjinlines.h"
 
 #include "vm/String-inl.h"
 
 using namespace js;
 using namespace js::gc;
 
 using mozilla::ArrayEnd;
 using mozilla::ArrayLength;
@@ -220,27 +221,22 @@ AtomIsInterned(JSContext *cx, JSAtom *at
 
     AtomSet::Ptr p = cx->runtime()->atoms().lookup(atom);
     if (!p)
         return false;
 
     return p->isTagged();
 }
 
-enum OwnCharsBehavior
-{
-    CopyChars, /* in other words, do not take ownership */
-    TakeCharOwnership
-};
-
 /*
  * When the jschars reside in a freshly allocated buffer the memory can be used
  * as a new JSAtom's storage without copying. The contract is that the caller no
  * longer owns the memory and this method is responsible for freeing the memory.
  */
+template <AllowGC allowGC>
 JS_ALWAYS_INLINE
 static JSAtom *
 AtomizeAndTakeOwnership(ExclusiveContext *cx, jschar *tbchars, size_t length, InternBehavior ib)
 {
     JS_ASSERT(tbchars[length] == 0);
 
     if (JSAtom *s = cx->staticStrings().lookup(tbchars, length)) {
         js_free(tbchars);
@@ -262,17 +258,17 @@ AtomizeAndTakeOwnership(ExclusiveContext
         JSAtom *atom = p->asPtr();
         p->setTagged(bool(ib));
         js_free(tbchars);
         return atom;
     }
 
     AutoCompartment ac(cx, cx->atomsCompartment());
 
-    JSFlatString *flat = js_NewString<CanGC>(cx, tbchars, length);
+    JSFlatString *flat = js_NewString<allowGC>(cx, tbchars, length);
     if (!flat) {
         js_free(tbchars);
         return NULL;
     }
 
     JSAtom *atom = flat->morphAtomizedStringIntoAtom();
 
     if (!atoms.relookupOrAdd(p, AtomHasher::Lookup(tbchars, length),
@@ -369,18 +365,19 @@ js::AtomizeString(ExclusiveContext *cx, 
 }
 
 template JSAtom *
 js::AtomizeString<CanGC>(ExclusiveContext *cx, JSString *str, InternBehavior ib);
 
 template JSAtom *
 js::AtomizeString<NoGC>(ExclusiveContext *cx, JSString *str, InternBehavior ib);
 
+template <AllowGC allowGC>
 JSAtom *
-js::Atomize(ExclusiveContext *cx, const char *bytes, size_t length, InternBehavior ib)
+js::AtomizeMaybeGC(ExclusiveContext *cx, const char *bytes, size_t length, InternBehavior ib)
 {
     CHECK_REQUEST(cx);
 
     if (!JSString::validateLength(cx, length))
         return NULL;
 
     static const unsigned ATOMIZE_BUF_MAX = 32;
     if (length < ATOMIZE_BUF_MAX) {
@@ -393,23 +390,37 @@ js::Atomize(ExclusiveContext *cx, const 
          */
         jschar inflated[ATOMIZE_BUF_MAX];
         size_t inflatedLength = ATOMIZE_BUF_MAX - 1;
         if (!InflateStringToBuffer(cx->maybeJSContext(),
                                    bytes, length, inflated, &inflatedLength))
         {
             return NULL;
         }
-        return AtomizeAndCopyChars<CanGC>(cx, inflated, inflatedLength, ib);
+        return AtomizeAndCopyChars<allowGC>(cx, inflated, inflatedLength, ib);
     }
 
     jschar *tbcharsZ = InflateString(cx, bytes, &length);
     if (!tbcharsZ)
         return NULL;
-    return AtomizeAndTakeOwnership(cx, tbcharsZ, length, ib);
+    return AtomizeAndTakeOwnership<allowGC>(cx, tbcharsZ, length, ib);
+}
+
+template JSAtom *
+js::AtomizeMaybeGC<CanGC>(ExclusiveContext *cx, const char *bytes, size_t length,
+                          InternBehavior ib);
+
+template JSAtom *
+js::AtomizeMaybeGC<NoGC>(ExclusiveContext *cx, const char *bytes, size_t length,
+                         InternBehavior ib);
+
+JSAtom *
+js::Atomize(ExclusiveContext *cx, const char *bytes, size_t length, InternBehavior ib)
+{
+    return AtomizeMaybeGC<CanGC>(cx, bytes, length, ib);
 }
 
 template <AllowGC allowGC>
 JSAtom *
 js::AtomizeChars(ExclusiveContext *cx, const jschar *chars, size_t length, InternBehavior ib)
 {
     CHECK_REQUEST(cx);
 
@@ -448,26 +459,50 @@ js::IndexToIdSlow(ExclusiveContext *cx, 
 
 template bool
 js::IndexToIdSlow<CanGC>(ExclusiveContext *cx, uint32_t index, MutableHandleId idp);
 
 template bool
 js::IndexToIdSlow<NoGC>(ExclusiveContext *cx, uint32_t index, FakeMutableHandle<jsid> idp);
 
 template <AllowGC allowGC>
+static JSAtom *
+ToAtomSlow(ExclusiveContext *cx, typename MaybeRooted<Value, allowGC>::HandleType arg)
+{
+    JS_ASSERT(!arg.isString());
+
+    Value v = arg;
+    if (!v.isPrimitive()) {
+        if (!cx->shouldBeJSContext() || !allowGC)
+            return NULL;
+        RootedValue v2(cx, v);
+        if (!ToPrimitive(cx->asJSContext(), JSTYPE_STRING, &v2))
+            return NULL;
+        v = v2;
+    }
+
+    if (v.isString())
+        return AtomizeString<allowGC>(cx, v.toString());
+    if (v.isInt32())
+        return Int32ToAtom<allowGC>(cx, v.toInt32());
+    if (v.isDouble())
+        return NumberToAtom<allowGC>(cx, v.toDouble());
+    if (v.isBoolean())
+        return v.toBoolean() ? cx->names().true_ : cx->names().false_;
+    if (v.isNull())
+        return cx->names().null;
+    return cx->names().undefined;
+}
+
+template <AllowGC allowGC>
 JSAtom *
 js::ToAtom(ExclusiveContext *cx, typename MaybeRooted<Value, allowGC>::HandleType v)
 {
-    if (!v.isString()) {
-        JSString *str = js::ToStringSlow<allowGC>(cx, v);
-        if (!str)
-            return NULL;
-        JS::Anchor<JSString *> anchor(str);
-        return AtomizeString<allowGC>(cx, str);
-    }
+    if (!v.isString())
+        return ToAtomSlow<allowGC>(cx, v);
 
     JSString *str = v.toString();
     if (str->isAtom())
         return &str->asAtom();
 
     JS::Anchor<JSString *> anchor(str);
     return AtomizeString<allowGC>(cx, str);
 }
--- a/js/src/jsatom.h
+++ b/js/src/jsatom.h
@@ -194,16 +194,21 @@ FinishCommonNames(JSRuntime *rt);
 
 /* N.B. must correspond to boolean tagging behavior. */
 enum InternBehavior
 {
     DoNotInternAtom = false,
     InternAtom = true
 };
 
+template <AllowGC allowGC>
+extern JSAtom *
+AtomizeMaybeGC(ExclusiveContext *cx, const char *bytes, size_t length,
+               js::InternBehavior ib = js::DoNotInternAtom);
+
 extern JSAtom *
 Atomize(ExclusiveContext *cx, const char *bytes, size_t length,
         js::InternBehavior ib = js::DoNotInternAtom);
 
 template <AllowGC allowGC>
 extern JSAtom *
 AtomizeChars(ExclusiveContext *cx, const jschar *chars, size_t length,
              js::InternBehavior ib = js::DoNotInternAtom);
--- a/js/src/jsnum.cpp
+++ b/js/src/jsnum.cpp
@@ -532,64 +532,123 @@ ToCStringBuf::ToCStringBuf() :dbuf(NULL)
     JS_STATIC_ASSERT(sbufSize >= DTOSTR_STANDARD_BUFFER_SIZE);
 }
 
 ToCStringBuf::~ToCStringBuf()
 {
     js_free(dbuf);
 }
 
+JS_ALWAYS_INLINE
+static JSFlatString *
+LookupDtoaCache(ThreadSafeContext *cx, double d)
+{
+    if (!cx->isExclusiveContext())
+        return NULL;
+
+    if (JSCompartment *comp = cx->asExclusiveContext()->compartment()) {
+        if (JSFlatString *str = comp->dtoaCache.lookup(10, d))
+            return str;
+    }
+
+    return NULL;
+}
+
+JS_ALWAYS_INLINE
+static void
+CacheNumber(ThreadSafeContext *cx, double d, JSFlatString *str)
+{
+    if (!cx->isExclusiveContext())
+        return;
+
+    if (JSCompartment *comp = cx->asExclusiveContext()->compartment())
+        comp->dtoaCache.cache(10, d, str);
+}
+
+JS_ALWAYS_INLINE
+static JSFlatString *
+LookupInt32ToString(ThreadSafeContext *cx, int32_t si)
+{
+    if (si >= 0 && StaticStrings::hasInt(si))
+        return cx->staticStrings().getInt(si);
+
+    return LookupDtoaCache(cx, si);
+}
+
+template <typename T>
+JS_ALWAYS_INLINE
+static T *
+BackfillInt32InBuffer(int32_t si, T *buffer, size_t size, size_t *length)
+{
+    uint32_t ui = mozilla::Abs(si);
+    JS_ASSERT_IF(si == INT32_MIN, ui == uint32_t(INT32_MAX) + 1);
+
+    RangedPtr<T> end(buffer + size - 1, buffer, size);
+    *end = '\0';
+    RangedPtr<T> start = BackfillIndexInCharBuffer(ui, end);
+    if (si < 0)
+        *--start = '-';
+
+    *length = end - start;
+    return start.get();
+}
+
 template <AllowGC allowGC>
 JSFlatString *
 js::Int32ToString(ThreadSafeContext *cx, int32_t si)
 {
-    uint32_t ui;
-    if (si >= 0) {
-        if (StaticStrings::hasInt(si))
-            return cx->staticStrings().getInt(si);
-        ui = si;
-    } else {
-        ui = uint32_t(-si);
-        JS_ASSERT_IF(si == INT32_MIN, ui == uint32_t(INT32_MAX) + 1);
-    }
-
-    JSCompartment *comp = cx->isExclusiveContext()
-                          ? cx->asExclusiveContext()->compartment()
-                          : NULL;
-    if (comp) {
-        if (JSFlatString *str = comp->dtoaCache.lookup(10, si))
-            return str;
-    }
+    if (JSFlatString *str = LookupInt32ToString(cx, si))
+        return str;
 
     JSShortString *str = js_NewGCShortString<allowGC>(cx);
     if (!str)
         return NULL;
 
     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 = '-';
+    size_t length;
+    jschar *start = BackfillInt32InBuffer(si, buffer,
+                                          JSShortString::MAX_SHORT_LENGTH + 1, &length);
 
-    jschar *dst = str->init(end - start);
-    PodCopy(dst, start.get(), end - start + 1);
+    PodCopy(str->init(length), start, length + 1);
 
-    if (comp)
-        comp->dtoaCache.cache(10, si, str);
+    CacheNumber(cx, si, str);
     return str;
 }
 
 template JSFlatString *
 js::Int32ToString<CanGC>(ThreadSafeContext *cx, int32_t si);
 
 template JSFlatString *
 js::Int32ToString<NoGC>(ThreadSafeContext *cx, int32_t si);
 
+template <AllowGC allowGC>
+JSAtom *
+js::Int32ToAtom(ExclusiveContext *cx, int32_t si)
+{
+    if (JSFlatString *str = LookupInt32ToString(cx, si))
+        return js::AtomizeString<allowGC>(cx, str);
+
+    char buffer[JSShortString::MAX_SHORT_LENGTH + 1];
+    size_t length;
+    char *start = BackfillInt32InBuffer(si, buffer, JSShortString::MAX_SHORT_LENGTH + 1, &length);
+
+    JSAtom *atom = AtomizeMaybeGC<allowGC>(cx, start, length);
+    if (!atom)
+        return NULL;
+
+    CacheNumber(cx, si, atom);
+    return atom;
+}
+
+template JSAtom *
+js::Int32ToAtom<CanGC>(ExclusiveContext *cx, int32_t si);
+
+template JSAtom *
+js::Int32ToAtom<NoGC>(ExclusiveContext *cx, int32_t si);
+
 /* Returns a non-NULL pointer to inside cbuf.  */
 static char *
 IntToCString(ToCStringBuf *cbuf, int i, int base = 10)
 {
     unsigned u = (i < 0) ? -i : i;
 
     RangedPtr<char> cp(cbuf->sbuf + cbuf->sbufSize - 1, cbuf->sbuf, cbuf->sbufSize);
     *cp = '\0';
@@ -1335,16 +1394,51 @@ js_NumberToString(ThreadSafeContext *cx,
 }
 
 template JSString *
 js_NumberToString<CanGC>(ThreadSafeContext *cx, double d);
 
 template JSString *
 js_NumberToString<NoGC>(ThreadSafeContext *cx, double d);
 
+template <AllowGC allowGC>
+JSAtom *
+js::NumberToAtom(ExclusiveContext *cx, double d)
+{
+    int32_t si;
+    if (mozilla::DoubleIsInt32(d, &si))
+        return Int32ToAtom<allowGC>(cx, si);
+
+    if (JSFlatString *str = LookupDtoaCache(cx, d))
+        return AtomizeString<allowGC>(cx, str);
+
+    ToCStringBuf cbuf;
+    char *numStr = FracNumberToCString(cx, &cbuf, d);
+    if (!numStr) {
+        js_ReportOutOfMemory(cx);
+        return NULL;
+    }
+    JS_ASSERT(!cbuf.dbuf && numStr >= cbuf.sbuf && numStr < cbuf.sbuf + cbuf.sbufSize);
+
+    size_t length = strlen(numStr);
+    JSAtom *atom = AtomizeMaybeGC<allowGC>(cx, numStr, length);
+    if (!atom)
+        return NULL;
+
+    CacheNumber(cx, d, atom);
+
+    return atom;
+}
+
+template JSAtom *
+js::NumberToAtom<CanGC>(ExclusiveContext *cx, double d);
+
+template JSAtom *
+js::NumberToAtom<NoGC>(ExclusiveContext *cx, double d);
+
 JSFlatString *
 js::NumberToString(JSContext *cx, double d)
 {
     if (JSString *str = js_NumberToStringWithBase<CanGC>(cx, d, 10))
         return &str->asFlat();
     return NULL;
 }
 
--- a/js/src/jsnum.h
+++ b/js/src/jsnum.h
@@ -37,31 +37,41 @@ js_InitNumberClass(JSContext *cx, js::Ha
 /*
  * String constants for global function names, used in jsapi.c and jsnum.c.
  */
 extern const char js_isNaN_str[];
 extern const char js_isFinite_str[];
 extern const char js_parseFloat_str[];
 extern const char js_parseInt_str[];
 
+class JSAtom;
+
 /*
  * When base == 10, this function implements ToString() as specified by
  * ECMA-262-5 section 9.8.1; but note that it handles integers specially for
  * performance.  See also js::NumberToCString().
  */
 template <js::AllowGC allowGC>
 extern JSString *
 js_NumberToString(js::ThreadSafeContext *cx, double d);
 
 namespace js {
 
+template <js::AllowGC allowGC>
+extern JSAtom *
+NumberToAtom(js::ExclusiveContext *cx, double d);
+
 template <AllowGC allowGC>
 extern JSFlatString *
 Int32ToString(ThreadSafeContext *cx, int32_t i);
 
+template <AllowGC allowGC>
+extern JSAtom *
+Int32ToAtom(ExclusiveContext *cx, int32_t si);
+
 /*
  * Convert an integer or double (contained in the given value) to a string and
  * append to the given buffer.
  */
 extern bool JS_FASTCALL
 NumberValueToStringBuffer(JSContext *cx, const Value &v, StringBuffer &sb);
 
 /* Same as js_NumberToString, different signature. */