Bug 938615 - Simplify usage of ScriptSource::chars (r=benjamin)

--HG--
extra : rebase_source : ae8644e441bd99e08c1743c5dc47517bae479f78

js/src/jsfun.cpp

@@ -1119,14 +1119,15 @@ JSFunction::createScriptForLazilyInterpretedFunction(JSContext *cx, HandleFuncti
         if (cx->zone()->needsBarrier())
             LazyScript::writeBarrierPre(lazy);
 
-        // Suppress GC when lazily compiling functions, to preserve source
-        // character buffers.
+        // Suppress GC for now although we should be able to remove this by
+        // making 'lazy' a Rooted<LazyScript*> (which requires adding a
+        // THING_ROOT_LAZY_SCRIPT).
         AutoSuppressGC suppressGC(cx);
 
         fun->flags &= ~INTERPRETED_LAZY;
         fun->flags |= INTERPRETED;
 
-        JSScript *script = lazy->maybeScript();
+        RootedScript script(cx, lazy->maybeScript());
 
         if (script) {
             fun->initScript(script);
@@ -1155,13 +1156,12 @@ JSFunction::createScriptForLazilyInterpretedFunction(JSContext *cx, HandleFuncti
         // has started.
         if (!lazy->numInnerFunctions() && !JS::IsIncrementalGCInProgress(cx->runtime())) {
             LazyScriptCache::Lookup lookup(cx, lazy);
-            cx->runtime()->lazyScriptCache.lookup(lookup, &script);
+            cx->runtime()->lazyScriptCache.lookup(lookup, script.address());
         }
 
         if (script) {
             RootedObject enclosingScope(cx, lazy->enclosingScope());
-            RootedScript scriptRoot(cx, script);
-            RootedScript clonedScript(cx, CloneScript(cx, enclosingScope, fun, scriptRoot));
+            RootedScript clonedScript(cx, CloneScript(cx, enclosingScope, fun, script));
             if (!clonedScript) {
                 fun->initLazyScript(lazy);
                 return false;
@@ -1182,7 +1182,8 @@ JSFunction::createScriptForLazilyInterpretedFunction(JSContext *cx, HandleFuncti
         JS_ASSERT(lazy->source()->hasSourceData());
 
         // Parse and compile the script from source.
-        const jschar *chars = lazy->source()->chars(cx);
+        SourceDataCache::AutoSuppressPurge asp(cx);
+        const jschar *chars = lazy->source()->chars(cx, asp);
         if (!chars) {
             fun->initLazyScript(lazy);
             return false;

js/src/jsscript.cpp

@@ -1032,9 +1032,22 @@ JSScript::sourceData(JSContext *cx)
     return scriptSource()->substring(cx, sourceStart, sourceEnd);
 }
 
-const jschar *
-SourceDataCache::lookup(ScriptSource *ss)
+SourceDataCache::AutoSuppressPurge::AutoSuppressPurge(JSContext *cx)
+ : cache_(cx->runtime()->sourceDataCache)
 {
+    oldValue_ = cache_.numSuppressPurges_++;
+}
+
+SourceDataCache::AutoSuppressPurge::~AutoSuppressPurge()
+{
+    cache_.numSuppressPurges_--;
+    JS_ASSERT(cache_.numSuppressPurges_ == oldValue_);
+}
+
+const jschar *
+SourceDataCache::lookup(ScriptSource *ss, const AutoSuppressPurge &asp)
+{
+    JS_ASSERT(this == &asp.cache());
     if (!map_)
         return nullptr;
     if (Map::Ptr p = map_->lookup(ss))
@@ -1043,8 +1056,10 @@ SourceDataCache::lookup(ScriptSource *ss)
 }
 
 bool
-SourceDataCache::put(ScriptSource *ss, const jschar *str)
+SourceDataCache::put(ScriptSource *ss, const jschar *str, const AutoSuppressPurge &asp)
 {
+    JS_ASSERT(this == &asp.cache());
+
     if (!map_) {
         map_ = js_new<Map>();
         if (!map_)
@@ -1063,7 +1078,7 @@ SourceDataCache::put(ScriptSource *ss, const jschar *str)
 void
 SourceDataCache::purge()
 {
-    if (!map_)
+    if (!map_ || numSuppressPurges_ > 0)
         return;
 
     for (Map::Range r = map_->all(); !r.empty(); r.popFront())
@@ -1074,7 +1089,7 @@ SourceDataCache::purge()
 }
 
 const jschar *
-ScriptSource::chars(JSContext *cx)
+ScriptSource::chars(JSContext *cx, const SourceDataCache::AutoSuppressPurge &asp)
 {
     if (const jschar *chars = getOffThreadCompressionChars(cx))
         return chars;
@@ -1082,7 +1097,7 @@ ScriptSource::chars(JSContext *cx)
 
 #ifdef USE_ZLIB
     if (compressed()) {
-        if (const jschar *decompressed = cx->runtime()->sourceDataCache.lookup(this))
+        if (const jschar *decompressed = cx->runtime()->sourceDataCache.lookup(this, asp))
             return decompressed;
       
         const size_t nbytes = sizeof(jschar) * (length_ + 1);
@@ -1099,7 +1114,7 @@ ScriptSource::chars(JSContext *cx)
 
         decompressed[length_] = 0;
 
-        if (!cx->runtime()->sourceDataCache.put(this, decompressed)) {
+        if (!cx->runtime()->sourceDataCache.put(this, decompressed, asp)) {
             JS_ReportOutOfMemory(cx);
             js_free(decompressed);
             return nullptr;
@@ -1115,7 +1130,8 @@ JSStableString *
 ScriptSource::substring(JSContext *cx, uint32_t start, uint32_t stop)
 {
     JS_ASSERT(start <= stop);
-    const jschar *chars = this->chars(cx);
+    SourceDataCache::AutoSuppressPurge asp(cx);
+    const jschar *chars = this->chars(cx, asp);
     if (!chars)
         return nullptr;
     JSFlatString *flatStr = js_NewStringCopyN<CanGC>(cx, chars + start, stop - start);
@@ -3150,14 +3166,13 @@ LazyScriptHashPolicy::match(JSScript *script, const Lookup &lookup)
         return false;
     }
 
-    // GC activity may destroy the character pointers being compared below.
-    AutoSuppressGC suppress(cx);
+    SourceDataCache::AutoSuppressPurge asp(cx);
 
-    const jschar *scriptChars = script->scriptSource()->chars(cx);
+    const jschar *scriptChars = script->scriptSource()->chars(cx, asp);
     if (!scriptChars)
         return false;
 
-    const jschar *lazyChars = lazy->source()->chars(cx);
+    const jschar *lazyChars = lazy->source()->chars(cx, asp);
     if (!lazyChars)
         return false;
 

js/src/jsscript.h

@@ -297,6 +297,36 @@ typedef HashMap<JSScript *,
                 DefaultHasher<JSScript *>,
                 SystemAllocPolicy> DebugScriptMap;
 
+class ScriptSource;
+
+class SourceDataCache
+{
+    typedef HashMap<ScriptSource *,
+                    const jschar *,
+                    DefaultHasher<ScriptSource *>,
+                    SystemAllocPolicy> Map;
+    Map *map_;
+    size_t numSuppressPurges_;
+
+  public:
+    SourceDataCache() : map_(nullptr), numSuppressPurges_(0) {}
+
+    class AutoSuppressPurge
+    {
+        SourceDataCache &cache_;
+        mozilla::DebugOnly<size_t> oldValue_;
+      public:
+        explicit AutoSuppressPurge(JSContext *cx);
+        ~AutoSuppressPurge();
+        SourceDataCache &cache() const { return cache_; }
+    };
+
+    const jschar *lookup(ScriptSource *ss, const AutoSuppressPurge &asp);
+    bool put(ScriptSource *ss, const jschar *chars, const AutoSuppressPurge &asp);
+
+    void purge();
+};
+
 class ScriptSource
 {
     friend class SourceCompressionTask;
@@ -369,7 +399,7 @@ class ScriptSource
         JS_ASSERT(hasSourceData());
         return argumentsNotIncluded_;
     }
-    const jschar *chars(JSContext *cx);
+    const jschar *chars(JSContext *cx, const SourceDataCache::AutoSuppressPurge &asp);
     JSStableString *substring(JSContext *cx, uint32_t start, uint32_t stop);
     size_t sizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);
 

js/src/vm/Runtime.h

@@ -167,21 +167,6 @@ struct ConservativeGCData
     }
 };
 
-class SourceDataCache
-{
-    typedef HashMap<ScriptSource *,
-                    const jschar *,
-                    DefaultHasher<ScriptSource *>,
-                    SystemAllocPolicy> Map;
-    Map *map_;
-
-  public:
-    SourceDataCache() : map_(nullptr) {}
-    const jschar *lookup(ScriptSource *ss);
-    bool put(ScriptSource *ss, const jschar *chars);
-    void purge();
-};
-
 struct EvalCacheEntry
 {
     JSScript *script;