Backed out changeset 75b28187fa5e (bug 1361900)

js/xpconnect/loader/ScriptCacheActors.cpp

@@ -31,16 +31,14 @@ ScriptCacheChild::Init(const Maybe<FileDescriptor>& cacheFile, bool wantCacheDat
 // Finalize the script cache for the content process, and send back data about
 // any scripts executed up to this point.
 void
-ScriptCacheChild::SendScriptsAndFinalize(ScriptPreloader::ScriptHash& scripts)
+ScriptCacheChild::Finalize(LinkedList<ScriptPreloader::CachedScript>& scripts)
 {
     MOZ_ASSERT(mWantCacheData);
 
     AutoSafeJSAPI jsapi;
 
-    auto matcher = ScriptPreloader::Match<ScriptPreloader::ScriptStatus::Saved>();
-
     nsTArray<ScriptData> dataArray;
-    for (auto& script : IterHash(scripts, matcher)) {
+    for (auto script : scripts) {
         if (!script->mSize && !script->XDREncode(jsapi.cx())) {
             continue;
         }

js/xpconnect/loader/ScriptCacheActors.h

@@ -49,7 +49,7 @@ public:
 protected:
     virtual void ActorDestroy(ActorDestroyReason aWhy) override;
 
-    void SendScriptsAndFinalize(ScriptPreloader::ScriptHash& scripts);
+    void Finalize(LinkedList<ScriptPreloader::CachedScript>& scripts);
 
 private:
     bool mWantCacheData = false;

js/xpconnect/loader/ScriptPreloader-inl.h

@@ -208,105 +208,6 @@ public:
     size_t cursor_ = 0;
 };
 
-
-template <typename T> struct Matcher;
-
-// Wraps the iterator for a nsTHashTable so that it may be used as a range
-// iterator. Each iterator result acts as a smart pointer to the hash element,
-// and has a Remove() method which will remove the element from the hash.
-//
-// It also accepts an optional Matcher instance against which to filter the
-// elements which should be iterated over.
-//
-// Example:
-//
-//    for (auto& elem : HashElemIter<HashType>(hash)) {
-//        if (elem->IsDead()) {
-//            elem.Remove();
-//        }
-//    }
-template <typename T>
-class HashElemIter
-{
-    using Iterator = typename T::Iterator;
-    using ElemType = typename T::UserDataType;
-
-    T& hash_;
-    Matcher<ElemType>* matcher_;
-    Maybe<Iterator> iter_;
-
-public:
-    explicit HashElemIter(T& hash, Matcher<ElemType>* matcher = nullptr)
-        : hash_(hash), matcher_(matcher)
-    {
-        iter_.emplace(Move(hash.Iter()));
-    }
-
-    class Elem
-    {
-        friend class HashElemIter<T>;
-
-        HashElemIter<T>& iter_;
-        bool done_;
-
-        Elem(HashElemIter& iter, bool done)
-            : iter_(iter), done_(done)
-        {}
-
-        Iterator& iter() { return iter_.iter_.ref(); }
-
-    public:
-        Elem& operator*() { return *this; }
-
-        ElemType get() { return done_ ? nullptr : iter().Data(); }
-
-        ElemType operator->() { return get(); }
-
-        operator ElemType() { return get(); }
-
-        void Remove() { iter().Remove(); }
-
-        Elem& operator++()
-        {
-            MOZ_ASSERT(!done_);
-
-            do {
-                iter().Next();
-                done_ = iter().Done();
-            } while (!done_ && iter_.matcher_ && !iter_.matcher_->Matches(get()));
-
-            return *this;
-        }
-
-        bool operator!=(Elem& other)
-        {
-            return done_ != other.done_ || this->get() != other.get();
-        }
-    };
-
-    Elem begin() { return Elem(*this, iter_->Done()); }
-
-    Elem end() { return Elem(*this, true); }
-};
-
-template <typename T>
-HashElemIter<T> IterHash(T& hash, Matcher<typename T::UserDataType>* matcher = nullptr)
-{
-    return HashElemIter<T>(hash, matcher);
-}
-
-template <typename T, typename F>
-bool
-Find(T&& iter, F&& match)
-{
-    for (auto& elem : iter) {
-        if (match(elem)) {
-            return true;
-        }
-    }
-    return false;
-}
-
 }; // namespace loader
 }; // namespace mozilla
 

js/xpconnect/loader/ScriptPreloader.cpp

@@ -12,7 +12,6 @@
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/FileUtils.h"
 #include "mozilla/Logging.h"
-#include "mozilla/ScopeExit.h"
 #include "mozilla/Services.h"
 #include "mozilla/Unused.h"
 #include "mozilla/dom/ContentChild.h"
@@ -56,13 +55,13 @@ ScriptPreloader::CollectReports(nsIHandleReportCallback* aHandleReport,
 {
     MOZ_COLLECT_REPORT(
         "explicit/script-preloader/heap/saved-scripts", KIND_HEAP, UNITS_BYTES,
-        SizeOfHashEntries<ScriptStatus::Saved>(mScripts, MallocSizeOf),
+        SizeOfLinkedList(mSavedScripts, MallocSizeOf),
         "Memory used to hold the scripts which have been executed in this "
         "session, and will be written to the startup script cache file.");
 
     MOZ_COLLECT_REPORT(
         "explicit/script-preloader/heap/restored-scripts", KIND_HEAP, UNITS_BYTES,
-        SizeOfHashEntries<ScriptStatus::Restored>(mScripts, MallocSizeOf),
+        SizeOfLinkedList(mRestoredScripts, MallocSizeOf),
         "Memory used to hold the scripts which have been restored from the "
         "startup script cache file, but have not been executed in this session.");
 
@@ -195,7 +194,11 @@ TraceOp(JSTracer* trc, void* data)
 void
 ScriptPreloader::Trace(JSTracer* trc)
 {
-    for (auto& script : IterHash(mScripts)) {
+    for (auto script : mSavedScripts) {
+        JS::TraceEdge(trc, &script->mScript, "ScriptPreloader::CachedScript.mScript");
+    }
+
+    for (auto script : mRestoredScripts) {
         JS::TraceEdge(trc, &script->mScript, "ScriptPreloader::CachedScript.mScript");
     }
 }
@@ -256,7 +259,8 @@ ScriptPreloader::Cleanup()
         }
     }
 
-    mScripts.Clear();
+    mSavedScripts.clear();
+    mRestoredScripts.clear();
 
     AutoSafeJSAPI jsapi;
     JS_RemoveExtraGCRootsTracer(jsapi.cx(), TraceOp, this);
@@ -265,19 +269,30 @@ ScriptPreloader::Cleanup()
 }
 
 void
-ScriptPreloader::FlushCache()
+ScriptPreloader::FlushScripts(LinkedList<CachedScript>& scripts)
 {
-    MonitorAutoLock mal(mMonitor);
+    for (auto next = scripts.getFirst(); next; ) {
+        auto script = next;
+        next = script->getNext();
 
-    for (auto& script : IterHash(mScripts)) {
         // We can only purge finished scripts here. Async scripts that are
         // still being parsed off-thread have a non-refcounted reference to
         // this script, which needs to stay alive until they finish parsing.
         if (script->mReadyToExecute) {
             script->Cancel();
-            script.Remove();
+            script->remove();
+            delete script;
         }
     }
+}
+
+void
+ScriptPreloader::FlushCache()
+{
+    MonitorAutoLock mal(mMonitor);
+
+    FlushScripts(mSavedScripts);
+    FlushScripts(mRestoredScripts);
 
     // If we've already finished saving the cache at this point, start a new
     // delayed save operation. This will write out an empty cache file in place
@@ -315,7 +330,7 @@ ScriptPreloader::Observe(nsISupports* subject, const char* topic, const char16_t
         mStartupFinished = true;
 
         if (mChildActor) {
-            mChildActor->SendScriptsAndFinalize(mScripts);
+            mChildActor->Finalize(mSavedScripts);
         }
     } else if (!strcmp(topic, SHUTDOWN_TOPIC)) {
         ForceWriteCacheFile();
@@ -434,9 +449,7 @@ ScriptPreloader::InitCacheInternal()
     }
 
     {
-        auto cleanup = MakeScopeExit([&] () {
+        AutoCleanLinkedList<CachedScript> scripts;
-            mScripts.Clear();
-        });
 
         Range<uint8_t> header(data, data + headerSize);
         data += headerSize;
@@ -461,14 +474,17 @@ ScriptPreloader::InitCacheInternal()
 
             script->mXDRRange.emplace(scriptData, scriptData + script->mSize);
 
-            mScripts.Put(script->mCachePath, script.release());
+            scripts.insertBack(script.release());
         }
 
         if (buf.error()) {
             return Err(NS_ERROR_UNEXPECTED);
         }
 
-        cleanup.release();
+        for (auto script : scripts) {
+            mScripts.Put(script->mCachePath, script);
+        }
+        mRestoredScripts = Move(scripts);
     }
 
     AutoJSAPI jsapi;
@@ -479,8 +495,7 @@ ScriptPreloader::InitCacheInternal()
     LOG(Info, "Off-thread decoding scripts...\n");
 
     JS::CompileOptions options(cx, JSVERSION_LATEST);
-
+    for (auto script : mRestoredScripts) {
-    for (auto& script : IterHash(mScripts, Match<ScriptStatus::Restored>())) {
         // Only async decode scripts which have been used in this process type.
         if (script->mProcessTypes.contains(CurrentProcessType()) &&
             script->AsyncDecodable() &&
@@ -521,25 +536,35 @@ ScriptPreloader::PrepareCacheWrite()
         return;
     }
 
-    bool found = Find(IterHash(mScripts), [] (CachedScript* script) {
+    if (mRestoredScripts.isEmpty()) {
-        return (script->mStatus == ScriptStatus::Restored ||
+        // Check for any new scripts that we need to save. If there aren't
-                !script->HasRange() || script->HasArray());
+        // any, and there aren't any saved scripts that we need to remove,
-    });
+        // don't bother writing out a new cache file.
-
+        bool found = false;
-    if (!found) {
+        for (auto script : mSavedScripts) {
-        mSaveComplete = true;
+            if (!script->HasRange() || script->HasArray()) {
-        return;
+                found = true;
+                break;
+            }
+        }
+        if (!found) {
+            mSaveComplete = true;
+            return;
+        }
     }
 
     AutoSafeJSAPI jsapi;
 
-    for (auto& script : IterHash(mScripts, Match<ScriptStatus::Saved>())) {
+    for (CachedScript* next = mSavedScripts.getFirst(); next; ) {
+        CachedScript* script = next;
+        next = script->getNext();
+
         // Don't write any scripts that are also in the child cache. They'll be
         // loaded from the child cache in that case, so there's no need to write
         // them twice.
         CachedScript* childScript = mChildCache ? mChildCache->mScripts.Get(script->mCachePath) : nullptr;
         if (childScript) {
-            if (childScript->mStatus == ScriptStatus::Saved) {
+            if (FindScript(mChildCache->mSavedScripts, script->mCachePath)) {
                 childScript->UpdateLoadTime(script->mLoadTime);
                 childScript->mProcessTypes += script->mProcessTypes;
             } else {
@@ -548,7 +573,8 @@ ScriptPreloader::PrepareCacheWrite()
         }
 
         if (childScript || (!script->mSize && !script->XDREncode(jsapi.cx()))) {
-            script.Remove();
+            script->remove();
+            delete script;
         } else {
             script->mSize = script->Range().length();
         }
@@ -603,7 +629,7 @@ ScriptPreloader::WriteCache()
     NS_TRY(cacheFile->OpenNSPRFileDesc(PR_WRONLY | PR_CREATE_FILE, 0644, &fd.rwget()));
 
     nsTArray<CachedScript*> scripts;
-    for (auto& script : IterHash(mScripts, Match<ScriptStatus::Saved>())) {
+    for (auto script : mSavedScripts) {
         scripts.AppendElement(script);
     }
 
@@ -664,6 +690,17 @@ ScriptPreloader::Run()
     return NS_OK;
 }
 
+/* static */ ScriptPreloader::CachedScript*
+ScriptPreloader::FindScript(LinkedList<CachedScript>& scripts, const nsCString& cachePath)
+{
+    for (auto script : scripts) {
+        if (script->mCachePath == cachePath) {
+            return script;
+        }
+    }
+    return nullptr;
+}
+
 void
 ScriptPreloader::NoteScript(const nsCString& url, const nsCString& cachePath,
                             JS::HandleScript jsscript)
@@ -680,14 +717,22 @@ ScriptPreloader::NoteScript(const nsCString& url, const nsCString& cachePath,
         return;
     }
 
-    auto script = mScripts.LookupOrAdd(cachePath, *this, url, cachePath, jsscript);
+    CachedScript* script = mScripts.Get(cachePath);
+    bool restored = script && FindScript(mRestoredScripts, cachePath);
 
-    if (script->mStatus == ScriptStatus::Restored) {
+    if (restored) {
-        script->mStatus = ScriptStatus::Saved;
+        script->remove();
+        mSavedScripts.insertBack(script);
 
         MOZ_ASSERT(jsscript);
         script->mScript = jsscript;
         script->mReadyToExecute = true;
+    } else if (!script) {
+        script = new CachedScript(*this, url, cachePath, jsscript);
+        mSavedScripts.insertBack(script);
+        mScripts.Put(cachePath, script);
+    } else {
+        return;
     }
 
     script->UpdateLoadTime(TimeStamp::Now());
@@ -699,13 +744,21 @@ ScriptPreloader::NoteScript(const nsCString& url, const nsCString& cachePath,
                             ProcessType processType, nsTArray<uint8_t>&& xdrData,
                             TimeStamp loadTime)
 {
-    auto script = mScripts.LookupOrAdd(cachePath, *this, url, cachePath, nullptr);
+    CachedScript* script = mScripts.Get(cachePath);
+    bool restored = script && FindScript(mRestoredScripts, cachePath);
 
-    if (script->mStatus == ScriptStatus::Restored) {
+    if (restored) {
-        script->mStatus = ScriptStatus::Saved;
+        script->remove();
+        mSavedScripts.insertBack(script);
 
         script->mReadyToExecute = true;
     } else {
+        if (!script) {
+            script = new CachedScript(this, url, cachePath, nullptr);
+            mSavedScripts.insertBack(script);
+            mScripts.Put(cachePath, script);
+        }
+
         if (!script->HasRange()) {
             MOZ_ASSERT(!script->HasArray());
 
@@ -816,7 +869,6 @@ ScriptPreloader::OffThreadDecodeCallback(void* token, void* context)
 
 ScriptPreloader::CachedScript::CachedScript(ScriptPreloader& cache, InputBuffer& buf)
     : mCache(cache)
-    , mStatus(ScriptStatus::Restored)
 {
     Code(buf);
 }

js/xpconnect/loader/ScriptPreloader.h

@@ -17,7 +17,7 @@
 #include "mozilla/Vector.h"
 #include "mozilla/Result.h"
 #include "mozilla/loader/AutoMemMap.h"
-#include "nsClassHashtable.h"
+#include "nsDataHashtable.h"
 #include "nsIFile.h"
 #include "nsIMemoryReporter.h"
 #include "nsIObserver.h"
@@ -43,12 +43,6 @@ namespace loader {
         Web,
         Extension,
     };
-
-    template <typename T>
-    struct Matcher
-    {
-        virtual bool Matches(T) = 0;
-    };
 }
 
 using namespace mozilla::loader;
@@ -107,11 +101,6 @@ protected:
     virtual ~ScriptPreloader() = default;
 
 private:
-    enum class ScriptStatus {
-      Restored,
-      Saved,
-    };
-
     // Represents a cached JS script, either initially read from the script
     // cache file, to be added to the next session's script cache file, or
     // both.
@@ -136,7 +125,7 @@ private:
     // the next session's cache file. If it was compiled in this session, its
     // mXDRRange will initially be empty, and its mXDRData buffer will be
     // populated just before it is written to the cache file.
-    class CachedScript
+    class CachedScript : public LinkedListElement<CachedScript>
     {
     public:
         CachedScript(CachedScript&&) = default;
@@ -145,14 +134,20 @@ private:
             : mCache(cache)
             , mURL(url)
             , mCachePath(cachePath)
-            , mStatus(ScriptStatus::Saved)
             , mScript(script)
             , mReadyToExecute(true)
         {}
 
         inline CachedScript(ScriptPreloader& cache, InputBuffer& buf);
 
-        ~CachedScript() = default;
+        ~CachedScript()
+        {
+#ifdef DEBUG
+            auto hashValue = mCache->mScripts.Get(mCachePath);
+            MOZ_ASSERT_IF(hashValue, hashValue == this);
+#endif
+            mCache->mScripts.Remove(mCachePath);
+        }
 
         // For use with nsTArray::Sort.
         //
@@ -181,18 +176,6 @@ private:
             }
         };
 
-        struct StatusMatcher final : public Matcher<CachedScript*>
-        {
-            StatusMatcher(ScriptStatus status) : mStatus(status) {}
-
-            virtual bool Matches(CachedScript* script)
-            {
-                return script->mStatus == mStatus;
-            }
-
-            const ScriptStatus mStatus;
-        };
-
         void Cancel();
 
         void FreeData()
@@ -291,8 +274,6 @@ private:
         // The size of this script's encoded XDR data.
         uint32_t mSize = 0;
 
-        ScriptStatus mStatus;
-
         TimeStamp mLoadTime{};
 
         JS::Heap<JSScript*> mScript;
@@ -320,13 +301,6 @@ private:
         MaybeOneOf<JS::TranscodeBuffer, nsTArray<uint8_t>> mXDRData;
     };
 
-    template <ScriptStatus status>
-    static Matcher<CachedScript*>* Match()
-    {
-        static CachedScript::StatusMatcher matcher{status};
-        return &matcher;
-    }
-
     // There's a trade-off between the time it takes to setup an off-thread
     // decode and the time we save by doing the decode off-thread. At this
     // point, the setup is quite expensive, and 20K is about where we start to
@@ -350,6 +324,7 @@ private:
     void Cleanup();
 
     void FlushCache();
+    void FlushScripts(LinkedList<CachedScript>& scripts);
 
     // Opens the cache file for reading.
     Result<Ok, nsresult> OpenCache();
@@ -366,6 +341,8 @@ private:
     Result<nsCOMPtr<nsIFile>, nsresult>
     GetCacheFile(const nsAString& suffix);
 
+    static CachedScript* FindScript(LinkedList<CachedScript>& scripts, const nsCString& cachePath);
+
     // Waits for the given cached script to finish compiling off-thread, or
     // decodes it synchronously on the main thread, as appropriate.
     JSScript* WaitForCachedScript(JSContext* cx, CachedScript* script);
@@ -382,19 +359,26 @@ private:
                 mallocSizeOf(mSaveThread.get()) + mallocSizeOf(mProfD.get()));
     }
 
-    using ScriptHash = nsClassHashtable<nsCStringHashKey, CachedScript>;
+    template<typename T>
-
+    static size_t SizeOfLinkedList(LinkedList<T>& list, mozilla::MallocSizeOf mallocSizeOf)
-    template<ScriptStatus status>
-    static size_t SizeOfHashEntries(ScriptHash& scripts, mozilla::MallocSizeOf mallocSizeOf)
     {
         size_t size = 0;
-        for (auto elem : IterHash(scripts, Match<status>())) {
+        for (auto elem : list) {
             size += elem->HeapSizeOfIncludingThis(mallocSizeOf);
         }
         return size;
     }
 
-    ScriptHash mScripts;
+    // The list of scripts executed during this session, and being saved for
+    // potential reuse, and to be written to the next session's cache file.
+    AutoCleanLinkedList<CachedScript> mSavedScripts;
+
+    // The list of scripts restored from the cache file at the start of this
+    // session. Scripts are removed from this list and moved to mSavedScripts
+    // the first time they're used during this session.
+    AutoCleanLinkedList<CachedScript> mRestoredScripts;
+
+    nsDataHashtable<nsCStringHashKey, CachedScript*> mScripts;
 
     // True after we've shown the first window, and are no longer adding new
     // scripts to the cache.