Bug 905382, part 2 - Implement per-zone string conversion cache. r=bz a=lsblakk
authorAndrew McCreight <continuation@gmail.com>
Mon, 25 Nov 2013 14:59:09 -0800
changeset 166608 f5a5f724bdc14a43ba3fb4d510990ee2868f7cc4
parent 166607 3e9fd7620fb68084bf6eba880126056023e2ea5e
child 166609 6034648945f3ad6b9f71b1d19188af8e9ca78bc0
push id3066
push userakeybl@mozilla.com
push dateMon, 09 Dec 2013 19:58:46 +0000
treeherdermozilla-beta@a31a0dce83aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, lsblakk
bugs905382
milestone27.0a2
Bug 905382, part 2 - Implement per-zone string conversion cache. r=bz a=lsblakk
js/xpconnect/src/XPCJSRuntime.cpp
js/xpconnect/src/XPCString.cpp
js/xpconnect/src/xpcpublic.h
xpcom/base/CycleCollectedJSRuntime.cpp
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -722,18 +722,16 @@ XPCJSRuntime::FinalizeCallback(JSFreeOp 
             // We add them to the array now and Release the array members
             // later to avoid the posibility of doing any JS GCThing
             // allocations during the gc cycle.
             self->mWrappedJSMap->FindDyingJSObjects(dyingWrappedJSArray);
 
             // Find dying scopes.
             XPCWrappedNativeScope::StartFinalizationPhaseOfGC(fop, self);
 
-            XPCStringConvert::ClearCache();
-
             self->mDoingFinalization = true;
             break;
         }
         case JSFINALIZE_GROUP_END:
         {
             MOZ_ASSERT(self->mDoingFinalization, "bad state");
             self->mDoingFinalization = false;
 
--- a/js/xpconnect/src/XPCString.cpp
+++ b/js/xpconnect/src/XPCString.cpp
@@ -19,34 +19,39 @@
  */
 
 #include "nscore.h"
 #include "nsString.h"
 #include "nsStringBuffer.h"
 #include "jsapi.h"
 #include "xpcpublic.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.
-nsStringBuffer* XPCStringConvert::sCachedBuffer = nullptr;
-JSString* XPCStringConvert::sCachedString = nullptr;
 
-// Called from GC finalize callback to make sure we don't hand out a pointer to
-// a JSString that's about to be finalized by incremental sweeping.
 // static
 void
-XPCStringConvert::ClearCache()
+XPCStringConvert::FreeZoneCache(JS::Zone *zone)
 {
-    sCachedBuffer = nullptr;
-    sCachedString = nullptr;
+    // Put the zone user data into an AutoPtr (which will do the cleanup for us),
+    // and null out the user data (which may already be null).
+    nsAutoPtr<ZoneStringCache> cache(static_cast<ZoneStringCache*>(JS_GetZoneUserData(zone)));
+    JS_SetZoneUserData(zone, nullptr);
 }
 
+// static
+void
+XPCStringConvert::ClearZoneCache(JS::Zone *zone)
+{
+    ZoneStringCache *cache = static_cast<ZoneStringCache*>(JS_GetZoneUserData(zone));
+    if (cache) {
+        cache->mBuffer = nullptr;
+        cache->mString = nullptr;
+    }
+}
+
+// static
 void
 XPCStringConvert::FinalizeDOMString(const JSStringFinalizer *fin, jschar *chars)
 {
     nsStringBuffer* buf = nsStringBuffer::FromData(chars);
     buf->Release();
 }
 
 const JSStringFinalizer XPCStringConvert::sDOMStringFinalizer =
--- a/js/xpconnect/src/xpcpublic.h
+++ b/js/xpconnect/src/xpcpublic.h
@@ -144,55 +144,71 @@ xpc_UnmarkSkippableJSHolders();
 // No JS can be on the stack when this is called. Probably only useful from
 // xpcshell.
 NS_EXPORT_(void)
 xpc_ActivateDebugMode();
 
 // readable string conversions, static methods and members only
 class XPCStringConvert
 {
+    // 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.
+    struct ZoneStringCache
+    {
+        nsStringBuffer* mBuffer;
+        JSString* mString;
+    };
+
 public:
 
     // If the string shares the readable's buffer, that buffer will
     // get assigned to *sharedBuffer.  Otherwise null will be
     // assigned.
     static jsval ReadableToJSVal(JSContext *cx, const nsAString &readable,
                                  nsStringBuffer** sharedBuffer);
 
     // Convert the given stringbuffer/length pair to a jsval
     static MOZ_ALWAYS_INLINE bool
     StringBufferToJSVal(JSContext* cx, nsStringBuffer* buf, uint32_t length,
                         JS::MutableHandleValue rval, bool* sharedBuffer)
     {
-        if (buf == sCachedBuffer &&
-            JS::GetGCThingZone(sCachedString) == js::GetContextZone(cx))
-        {
-            rval.set(JS::StringValue(sCachedString));
+        JS::Zone *zone = js::GetContextZone(cx);
+        ZoneStringCache *cache = static_cast<ZoneStringCache*>(JS_GetZoneUserData(zone));
+        if (cache && buf == cache->mBuffer) {
+            MOZ_ASSERT(JS::GetGCThingZone(cache->mString) == zone);
+            JS::MarkStringAsLive(zone, cache->mString);
+            rval.setString(cache->mString);
             *sharedBuffer = false;
             return true;
         }
 
         JSString *str = JS_NewExternalString(cx,
                                              static_cast<jschar*>(buf->Data()),
                                              length, &sDOMStringFinalizer);
         if (!str) {
             return false;
         }
         rval.setString(str);
-        sCachedString = str;
-        sCachedBuffer = buf;
+        if (!cache) {
+            cache = new ZoneStringCache();
+            JS_SetZoneUserData(zone, cache);
+        }
+        cache->mBuffer = buf;
+        cache->mString = str;
         *sharedBuffer = true;
         return true;
     }
 
-    static void ClearCache();
+    static void FreeZoneCache(JS::Zone *zone);
+    static void ClearZoneCache(JS::Zone *zone);
 
 private:
-    static nsStringBuffer* sCachedBuffer;
-    static JSString* sCachedString;
     static const JSStringFinalizer sDOMStringFinalizer;
 
     static void FinalizeDOMString(const JSStringFinalizer *fin, jschar *chars);
 
     XPCStringConvert();         // not implemented
 };
 
 namespace xpc {
--- a/xpcom/base/CycleCollectedJSRuntime.cpp
+++ b/xpcom/base/CycleCollectedJSRuntime.cpp
@@ -447,16 +447,18 @@ CycleCollectedJSRuntime::CycleCollectedJ
   }
 
   if (!JS_AddExtraGCRootsTracer(mJSRuntime, TraceBlackJS, this)) {
     MOZ_CRASH();
   }
   JS_SetGrayGCRootsTracer(mJSRuntime, TraceGrayJS, this);
   JS_SetGCCallback(mJSRuntime, GCCallback, this);
   JS_SetContextCallback(mJSRuntime, ContextCallback, this);
+  JS_SetDestroyZoneCallback(mJSRuntime, XPCStringConvert::FreeZoneCache);
+  JS_SetSweepZoneCallback(mJSRuntime, XPCStringConvert::ClearZoneCache);
 
   nsCycleCollector_registerJSRuntime(this);
 }
 
 void
 CycleCollectedJSRuntime::DestroyRuntime()
 {
   if (!mJSRuntime) {