Bug 1353758 - Move the external string cache into the JS engine and i…

…mprove it. r=arai,bz

js/src/builtin/TestingFunctions.cpp

@@ -1302,6 +1302,37 @@ NewExternalString(JSContext* cx, unsigned argc, Value* vp)
     return true;
 }
 
+static bool
+NewMaybeExternalString(JSContext* cx, unsigned argc, Value* vp)
+{
+    CallArgs args = CallArgsFromVp(argc, vp);
+
+    if (args.length() != 1 || !args[0].isString()) {
+        JS_ReportErrorASCII(cx, "newMaybeExternalString takes exactly one string argument.");
+        return false;
+    }
+
+    RootedString str(cx, args[0].toString());
+    size_t len = str->length();
+
+    UniqueTwoByteChars buf(cx->pod_malloc<char16_t>(len));
+    if (!buf)
+        return false;
+
+    if (!JS_CopyStringChars(cx, mozilla::Range<char16_t>(buf.get(), len), str))
+        return false;
+
+    bool allocatedExternal;
+    JSString* res = JS_NewMaybeExternalString(cx, buf.get(), len, &ExternalStringFinalizer,
+                                              &allocatedExternal);
+    if (!res)
+        return false;
+
+    mozilla::Unused << buf.release();
+    args.rval().setString(res);
+    return true;
+}
+
 static bool
 EnsureFlatString(JSContext* cx, unsigned argc, Value* vp)
 {
@@ -4354,6 +4385,10 @@ static const JSFunctionSpecWithHelp TestingFunctions[] = {
 "newExternalString(str)",
 "  Copies str's chars and returns a new external string."),
 
+    JS_FN_HELP("newMaybeExternalString", NewMaybeExternalString, 1, 0,
+"newMaybeExternalString(str)",
+"  Like newExternalString but uses the JS_NewMaybeExternalString API."),
+
     JS_FN_HELP("ensureFlatString", EnsureFlatString, 1, 0,
 "ensureFlatString(str)",
 "  Ensures str is a flat (null-terminated) string and returns it."),

js/src/gc/Zone.cpp

@@ -42,6 +42,7 @@ JS::Zone::Zone(JSRuntime* rt, ZoneGroup* group)
     typeDescrObjects_(group, this, SystemAllocPolicy()),
     markedAtoms_(group),
     atomCache_(group),
+    externalStringCache_(group),
     usage(&rt->gc.usage),
     threshold(),
     gcDelayBytes(0),

js/src/gc/Zone.h

@@ -88,6 +88,23 @@ template <typename T>
 class ZoneCellIter;
 
 } // namespace gc
+
+class MOZ_NON_TEMPORARY_CLASS ExternalStringCache
+{
+    static const size_t NumEntries = 4;
+    mozilla::Array<JSString*, NumEntries> entries_;
+
+    ExternalStringCache(const ExternalStringCache&) = delete;
+    void operator=(const ExternalStringCache&) = delete;
+
+  public:
+    ExternalStringCache() { purge(); }
+    void purge() { mozilla::PodArrayZero(entries_); }
+
+    MOZ_ALWAYS_INLINE JSString* lookup(const char16_t* chars, size_t len) const;
+    MOZ_ALWAYS_INLINE void put(JSString* s);
+};
+
 } // namespace js
 
 namespace JS {
@@ -438,11 +455,16 @@ struct Zone : public JS::shadow::Zone,
     // Set of atoms recently used by this Zone. Purged on GC.
     js::ZoneGroupOrGCTaskData<js::AtomSet> atomCache_;
 
+    // Cache storing allocated external strings. Purged on GC.
+    js::ZoneGroupOrGCTaskData<js::ExternalStringCache> externalStringCache_;
+
   public:
     js::SparseBitmap& markedAtoms() { return markedAtoms_.ref(); }
 
     js::AtomSet& atomCache() { return atomCache_.ref(); }
 
+    js::ExternalStringCache& externalStringCache() { return externalStringCache_.ref(); };
+
     // Track heap usage under this Zone.
     js::gc::HeapUsage usage;
 

js/src/jit-test/tests/basic/external-strings.js

@@ -34,3 +34,15 @@ function testQuote() {
     }
 }
 testQuote();
+
+function testMaybeExternal() {
+    for (var i=0; i<10; i++) {
+        var s = "abcdef4321" + i;
+        assertEq(newMaybeExternalString(s), s);
+        if ((i % 2) === 0)
+            assertEq(ensureFlatString(newMaybeExternalString(s)), s);
+    }
+}
+testMaybeExternal();
+gc();
+testMaybeExternal();

js/src/jsapi.cpp

@@ -676,18 +676,6 @@ JS_SetSizeOfIncludingThisCompartmentCallback(JSContext* cx,
     cx->runtime()->sizeOfIncludingThisCompartmentCallback = callback;
 }
 
-JS_PUBLIC_API(void)
-JS_SetDestroyZoneCallback(JSContext* cx, JSZoneCallback callback)
-{
-    cx->runtime()->destroyZoneCallback = callback;
-}
-
-JS_PUBLIC_API(void)
-JS_SetSweepZoneCallback(JSContext* cx, JSZoneCallback callback)
-{
-    cx->runtime()->sweepZoneCallback = callback;
-}
-
 JS_PUBLIC_API(void)
 JS_SetCompartmentNameCallback(JSContext* cx, JSCompartmentNameCallback callback)
 {
@@ -1501,11 +1489,11 @@ JS_NewExternalString(JSContext* cx, const char16_t* chars, size_t length,
 
 JS_PUBLIC_API(JSString*)
 JS_NewMaybeExternalString(JSContext* cx, const char16_t* chars, size_t length,
-                          const JSStringFinalizer* fin, bool* isExternal)
+                          const JSStringFinalizer* fin, bool* allocatedExternal)
 {
     AssertHeapIsIdle();
     CHECK_REQUEST(cx);
-    return NewMaybeExternalString(cx, chars, length, fin, isExternal);
+    return NewMaybeExternalString(cx, chars, length, fin, allocatedExternal);
 }
 
 extern JS_PUBLIC_API(bool)

js/src/jsapi.h

@@ -706,9 +706,6 @@ typedef size_t
 (* JSSizeOfIncludingThisCompartmentCallback)(mozilla::MallocSizeOf mallocSizeOf,
                                              JSCompartment* compartment);
 
-typedef void
-(* JSZoneCallback)(JS::Zone* zone);
-
 typedef void
 (* JSCompartmentNameCallback)(JSContext* cx, JSCompartment* compartment,
                               char* buf, size_t bufsize);
@@ -1323,12 +1320,6 @@ extern JS_PUBLIC_API(void)
 JS_SetSizeOfIncludingThisCompartmentCallback(JSContext* cx,
                                              JSSizeOfIncludingThisCompartmentCallback callback);
 
-extern JS_PUBLIC_API(void)
-JS_SetDestroyZoneCallback(JSContext* cx, JSZoneCallback callback);
-
-extern JS_PUBLIC_API(void)
-JS_SetSweepZoneCallback(JSContext* cx, JSZoneCallback callback);
-
 extern JS_PUBLIC_API(void)
 JS_SetCompartmentNameCallback(JSContext* cx, JSCompartmentNameCallback callback);
 
@@ -1856,14 +1847,14 @@ JS_NewExternalString(JSContext* cx, const char16_t* chars, size_t length,
 
 /**
  * Create a new JSString whose chars member may refer to external memory.
- * If the returned string refers to the external memory, |*isExternal| is set
- * to true.  Otherwise the returned string is not an external string and
- * |*isExternal| is set to false.  If |*isExternal| is false, |fin| won't be
- * called.
+ * If a new external string is allocated, |*allocatedExternal| is set to true.
+ * Otherwise the returned string is either not an external string or an
+ * external string allocated by a previous call and |*allocatedExternal| is set
+ * to false. If |*allocatedExternal| is false, |fin| won't be called.
  */
 extern JS_PUBLIC_API(JSString*)
 JS_NewMaybeExternalString(JSContext* cx, const char16_t* chars, size_t length,
-                          const JSStringFinalizer* fin, bool* isExternal);
+                          const JSStringFinalizer* fin, bool* allocatedExternal);
 
 /**
  * Return whether 'str' was created with JS_NewExternalString or

js/src/jsgc.cpp

@@ -2557,8 +2557,6 @@ GCRuntime::updateZonePointersToRelocatedCells(Zone* zone, AutoLockForExclusiveAc
     // Call callbacks to get the rest of the system to fixup other untraced pointers.
     for (CompartmentsInZoneIter comp(zone); !comp.done(); comp.next())
         callWeakPointerCompartmentCallbacks(comp);
-    if (rt->sweepZoneCallback)
-        rt->sweepZoneCallback(zone);
 }
 
 /*
@@ -3514,8 +3512,6 @@ Zone::sweepCompartments(FreeOp* fop, bool keepAtleastOne, bool destroyingRuntime
 void
 GCRuntime::sweepZones(FreeOp* fop, ZoneGroup* group, bool destroyingRuntime)
 {
-    JSZoneCallback callback = rt->destroyZoneCallback;
-
     Zone** read = group->zones().begin();
     Zone** end = group->zones().end();
     Zone** write = read;
@@ -3541,9 +3537,6 @@ GCRuntime::sweepZones(FreeOp* fop, ZoneGroup* group, bool destroyingRuntime)
                     arenasEmptyAtShutdown = false;
 #endif
 
-                if (callback)
-                    callback(zone);
-
                 zone->sweepCompartments(fop, false, destroyingRuntime);
                 MOZ_ASSERT(zone->compartments().empty());
                 MOZ_ASSERT_IF(arenasEmptyAtShutdown, zone->typeDescrObjects().empty());
@@ -3635,8 +3628,10 @@ GCRuntime::purgeRuntime(AutoLockForExclusiveAccess& lock)
     for (GCCompartmentsIter comp(rt); !comp.done(); comp.next())
         comp->purge();
 
-    for (GCZonesIter zone(rt); !zone.done(); zone.next())
+    for (GCZonesIter zone(rt); !zone.done(); zone.next()) {
         zone->atomCache().clearAndShrink();
+        zone->externalStringCache().purge();
+    }
 
     for (const CooperatingContext& target : rt->cooperatingContexts()) {
         freeUnusedLifoBlocksAfterSweeping(&target.context()->tempLifoAlloc());
@@ -5101,9 +5096,6 @@ GCRuntime::beginSweepingSweepGroup(AutoLockForExclusiveAccess& lock)
         if (zone->isAtomsZone())
             sweepingAtoms = true;
 
-        if (rt->sweepZoneCallback)
-            rt->sweepZoneCallback(zone);
-
 #ifdef DEBUG
         zone->gcLastSweepGroupIndex = sweepGroupIndex;
 #endif

js/src/vm/Runtime.cpp

@@ -115,8 +115,6 @@ JSRuntime::JSRuntime(JSRuntime* parentRuntime)
     allowRelazificationForTesting(false),
     destroyCompartmentCallback(nullptr),
     sizeOfIncludingThisCompartmentCallback(nullptr),
-    destroyZoneCallback(nullptr),
-    sweepZoneCallback(nullptr),
     compartmentNameCallback(nullptr),
     externalStringSizeofCallback(nullptr),
     securityCallbacks(&NullSecurityCallbacks),

js/src/vm/Runtime.h

@@ -497,12 +497,6 @@ struct JSRuntime : public js::MallocProvider<JSRuntime>
     /* Compartment memory reporting callback. */
     js::ActiveThreadData<JSSizeOfIncludingThisCompartmentCallback> sizeOfIncludingThisCompartmentCallback;
 
-    /* Zone destroy callback. */
-    js::ActiveThreadData<JSZoneCallback> destroyZoneCallback;
-
-    /* Zone sweep callback. */
-    js::ActiveThreadData<JSZoneCallback> sweepZoneCallback;
-
     /* Call this to get the name of a compartment. */
     js::ActiveThreadData<JSCompartmentNameCallback> compartmentNameCallback;
 

js/src/vm/String.cpp

@@ -26,6 +26,7 @@ using namespace js;
 
 using mozilla::IsSame;
 using mozilla::PodCopy;
+using mozilla::PodEqual;
 using mozilla::RangedPtr;
 using mozilla::RoundUpPow2;
 
@@ -1413,17 +1414,67 @@ NewStringCopyUTF8N(JSContext* cx, const JS::UTF8Chars utf8)
 template JSFlatString*
 NewStringCopyUTF8N<CanGC>(JSContext* cx, const JS::UTF8Chars utf8);
 
+MOZ_ALWAYS_INLINE JSString*
+ExternalStringCache::lookup(const char16_t* chars, size_t len) const
+{
+    AutoCheckCannotGC nogc;
+
+    for (size_t i = 0; i < NumEntries; i++) {
+        JSString* str = entries_[i];
+        if (!str || str->length() != len)
+            continue;
+
+        const char16_t* strChars = str->asLinear().nonInlineTwoByteChars(nogc);
+        if (chars == strChars) {
+            // Note that we don't need an incremental barrier here or below.
+            // The cache is purged on GC so any string we get from the cache
+            // must have been allocated after the GC started.
+            return str;
+        }
+
+        // Compare the chars. Don't do this for long strings as it will be
+        // faster to allocate a new external string.
+        static const size_t MaxLengthForCharComparison = 100;
+        if (len <= MaxLengthForCharComparison && PodEqual(chars, strChars, len))
+            return str;
+    }
+
+    return nullptr;
+}
+
+MOZ_ALWAYS_INLINE void
+ExternalStringCache::put(JSString* str)
+{
+    MOZ_ASSERT(str->isExternal());
+
+    for (size_t i = NumEntries - 1; i > 0; i--)
+        entries_[i] = entries_[i - 1];
+
+    entries_[0] = str;
+}
+
 JSString*
 NewMaybeExternalString(JSContext* cx, const char16_t* s, size_t n, const JSStringFinalizer* fin,
-                       bool* isExternal)
+                       bool* allocatedExternal)
 {
     if (JSString* str = TryEmptyOrStaticString(cx, s, n)) {
-        *isExternal = false;
+        *allocatedExternal = false;
+        return str;
+    }
+
+    ExternalStringCache& cache = cx->zone()->externalStringCache();
+    if (JSString* str = cache.lookup(s, n)) {
+        *allocatedExternal = false;
         return str;
     }
 
-    *isExternal = true;
-    return JSExternalString::new_(cx, s, n, fin);
+    JSString* str = JSExternalString::new_(cx, s, n, fin);
+    if (!str)
+        return nullptr;
+
+    *allocatedExternal = true;
+    cache.put(str);
+    return str;
 }
 
 } /* namespace js */

js/src/vm/String.h

@@ -1315,7 +1315,7 @@ NewStringCopyUTF8Z(JSContext* cx, const JS::ConstUTF8CharsZ utf8)
 
 JSString*
 NewMaybeExternalString(JSContext* cx, const char16_t* s, size_t n, const JSStringFinalizer* fin,
-                       bool* isExternal);
+                       bool* allocatedExternal);
 
 JS_STATIC_ASSERT(sizeof(HashNumber) == 4);