Bug 773520. Add a one-slot cache for converting stringbuffers to JSStrings. r=peterv,bhackett
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 28 Aug 2012 13:10:09 -0400
changeset 105714 9dbfbeaf89e385974a54a4bba7101a6b01419725
parent 105713 41b8acc383568b521ae0efd691ebd30ca92c10b1
child 105715 ca53a84f1a9439b66a8c631e389ca1f4684e7035
push id55
push usershu@rfrn.org
push dateThu, 30 Aug 2012 01:33:09 +0000
reviewerspeterv, bhackett
bugs773520
milestone18.0a1
Bug 773520. Add a one-slot cache for converting stringbuffers to JSStrings. r=peterv,bhackett
js/src/jsfriendapi.cpp
js/src/jsfriendapi.h
js/src/jsgc.h
js/xpconnect/src/XPCQuickStubs.cpp
js/xpconnect/src/XPCString.cpp
js/xpconnect/src/xpcpublic.h
--- a/js/src/jsfriendapi.cpp
+++ b/js/src/jsfriendapi.cpp
@@ -509,23 +509,16 @@ js::TraceWeakMaps(WeakMapTracer *trc)
 
 JS_FRIEND_API(bool)
 js::GCThingIsMarkedGray(void *thing)
 {
     JS_ASSERT(thing);
     return reinterpret_cast<gc::Cell *>(thing)->isMarked(gc::GRAY);
 }
 
-JS_FRIEND_API(JSCompartment*)
-js::GetGCThingCompartment(void *thing)
-{
-    JS_ASSERT(thing);
-    return reinterpret_cast<gc::Cell *>(thing)->compartment();
-}
-
 JS_FRIEND_API(void)
 js::VisitGrayWrapperTargets(JSCompartment *comp, GCThingCallback *callback, void *closure)
 {
     for (WrapperMap::Enum e(comp->crossCompartmentWrappers); !e.empty(); e.popFront()) {
         gc::Cell *thing = e.front().key.wrapped;
         if (thing->isMarked(gc::GRAY))
             callback(closure, thing);
     }
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -272,19 +272,16 @@ struct WeakMapTracer {
 };
 
 extern JS_FRIEND_API(void)
 TraceWeakMaps(WeakMapTracer *trc);
 
 extern JS_FRIEND_API(bool)
 GCThingIsMarkedGray(void *thing);
 
-extern JS_FRIEND_API(JSCompartment*)
-GetGCThingCompartment(void *thing);
-
 typedef void
 (GCThingCallback)(void *closure, void *gcthing);
 
 extern JS_FRIEND_API(void)
 VisitGrayWrapperTargets(JSCompartment *comp, GCThingCallback *callback, void *closure);
 
 /*
  * Shadow declarations of JS internal structures, for access by inline access
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@@ -1193,13 +1193,23 @@ MaybeVerifyBarriers(JSContext *cx, bool 
 {
 }
 
 #endif
 
 } /* namespace gc */
 
 static inline JSCompartment *
-GetObjectCompartment(JSObject *obj) { return reinterpret_cast<js::gc::Cell *>(obj)->compartment(); }
+GetGCThingCompartment(void *thing)
+{
+    JS_ASSERT(thing);
+    return reinterpret_cast<gc::Cell *>(thing)->compartment();
+}
+
+static inline JSCompartment *
+GetObjectCompartment(JSObject *obj)
+{
+    return GetGCThingCompartment(obj);
+}
 
 } /* namespace js */
 
 #endif /* jsgc_h___ */
--- a/js/xpconnect/src/XPCQuickStubs.cpp
+++ b/js/xpconnect/src/XPCQuickStubs.cpp
@@ -934,27 +934,16 @@ xpc_qsJsvalToWcharStr(JSContext *cx, jsv
 
     *pstr = static_cast<const PRUnichar *>(chars);
     return true;
 }
 
 namespace xpc {
 
 bool
-StringToJsval(JSContext *cx, nsAString &str, JS::Value *rval)
-{
-    // From the T_DOMSTRING case in XPCConvert::NativeData2JS.
-    if (str.IsVoid()) {
-        *rval = JSVAL_NULL;
-        return true;
-    }
-    return NonVoidStringToJsval(cx, str, rval);
-}
-
-bool
 NonVoidStringToJsval(JSContext *cx, nsAString &str, JS::Value *rval)
 {
     nsStringBuffer* sharedBuffer;
     jsval jsstr = XPCStringConvert::ReadableToJSVal(cx, str, &sharedBuffer);
     if (JSVAL_IS_NULL(jsstr))
         return false;
     *rval = jsstr;
     if (sharedBuffer) {
--- a/js/xpconnect/src/XPCString.cpp
+++ b/js/xpconnect/src/XPCString.cpp
@@ -16,20 +16,33 @@
  * Wrap the JSString with a root-holding XPCJSReadableStringWrapper, which roots
  * the string and exposes its buffer via the nsAString interface, as
  * well as providing refcounting support.
  */
 
 #include "xpcprivate.h"
 #include "nsStringBuffer.h"
 
+// One-slot cache, because it turns out it's common for web pages to
+// get the same string a few times in a row.  We get about a 40% cache
+// hit rate on this cache last it was measured.  We'd get about 70%
+// hit rate with a hashtable with removal on finalization, but that
+// would take a lot more machinery.
+static nsStringBuffer* sCachedBuffer = nullptr;
+static JSString* sCachedString = nullptr;
+
 static void
 FinalizeDOMString(const JSStringFinalizer *fin, jschar *chars)
 {
-    nsStringBuffer::FromData(chars)->Release();
+    nsStringBuffer* buf = nsStringBuffer::FromData(chars);
+    if (buf == sCachedBuffer) {
+        sCachedBuffer = nullptr;
+        // No need to clear sCachedString
+    }
+    buf->Release();
 }
 
 static const JSStringFinalizer sDOMStringFinalizer = { FinalizeDOMString };
 
 
 // convert a readable to a JSString, copying string data
 // static
 jsval
@@ -42,24 +55,32 @@ XPCStringConvert::ReadableToJSVal(JSCont
 
     uint32_t length = readable.Length();
 
     if (length == 0)
         return JS_GetEmptyStringValue(cx);
 
     nsStringBuffer *buf = nsStringBuffer::FromString(readable);
     if (buf) {
+        if (buf == sCachedBuffer &&
+            js::GetGCThingCompartment(sCachedString) == js::GetContextCompartment(cx)) {
+            // We're done.  Just return our existing string.
+            return JS::StringValue(sCachedString);
+        }
+
         // yay, we can share the string's buffer!
 
         str = JS_NewExternalString(cx,
                                    reinterpret_cast<jschar *>(buf->Data()),
                                    length, &sDOMStringFinalizer);
 
         if (str) {
             *sharedBuffer = buf;
+            sCachedString = str;
+            sCachedBuffer = buf;
         }
     } else {
         // blech, have to copy.
 
         jschar *chars = reinterpret_cast<jschar *>
                                         (JS_malloc(cx, (length + 1) *
                                                    sizeof(jschar)));
         if (!chars)
--- a/js/xpconnect/src/xpcpublic.h
+++ b/js/xpconnect/src/xpcpublic.h
@@ -236,18 +236,26 @@ bool DeferredRelease(nsISupports *obj);
 bool Base64Encode(JSContext *cx, JS::Value val, JS::Value *out);
 bool Base64Decode(JSContext *cx, JS::Value val, JS::Value *out);
 
 /**
  * Convert an nsString to jsval, returning true on success.
  * Note, the ownership of the string buffer may be moved from str to rval.
  * If that happens, str will point to an empty string after this call.
  */
-bool StringToJsval(JSContext *cx, nsAString &str, JS::Value *rval);
 bool NonVoidStringToJsval(JSContext *cx, nsAString &str, JS::Value *rval);
+inline bool StringToJsval(JSContext *cx, nsAString &str, JS::Value *rval)
+{
+    // From the T_DOMSTRING case in XPCConvert::NativeData2JS.
+    if (str.IsVoid()) {
+        *rval = JSVAL_NULL;
+        return true;
+    }
+    return NonVoidStringToJsval(cx, str, rval);
+}
 
 nsIPrincipal *GetCompartmentPrincipal(JSCompartment *compartment);
 
 void DumpJSHeap(FILE* file);
 
 void SetLocationForGlobal(JSObject *global, const nsACString& location);
 void SetLocationForGlobal(JSObject *global, nsIURI *locationURI);