From b5071f569a568c01ef0fb8f5d9f8a81fc370dc9d Mon Sep 17 00:00:00 2001 From: Sebastian Hengst Date: Sat, 13 May 2017 18:53:10 +0200 Subject: [PATCH] Backed out changeset 75b28187fa5e (bug 1361900) --- js/xpconnect/loader/ScriptCacheActors.cpp | 6 +- js/xpconnect/loader/ScriptCacheActors.h | 2 +- js/xpconnect/loader/ScriptPreloader-inl.h | 99 ----------------- js/xpconnect/loader/ScriptPreloader.cpp | 124 +++++++++++++++------- js/xpconnect/loader/ScriptPreloader.h | 68 +++++------- 5 files changed, 117 insertions(+), 182 deletions(-) diff --git a/js/xpconnect/loader/ScriptCacheActors.cpp b/js/xpconnect/loader/ScriptCacheActors.cpp index fd52617e50f7..b72c58c0b203 100644 --- a/js/xpconnect/loader/ScriptCacheActors.cpp +++ b/js/xpconnect/loader/ScriptCacheActors.cpp @@ -31,16 +31,14 @@ ScriptCacheChild::Init(const Maybe& 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& scripts) { MOZ_ASSERT(mWantCacheData); AutoSafeJSAPI jsapi; - auto matcher = ScriptPreloader::Match(); - nsTArray dataArray; - for (auto& script : IterHash(scripts, matcher)) { + for (auto script : scripts) { if (!script->mSize && !script->XDREncode(jsapi.cx())) { continue; } diff --git a/js/xpconnect/loader/ScriptCacheActors.h b/js/xpconnect/loader/ScriptCacheActors.h index 3db95f257a8c..3572edb28074 100644 --- a/js/xpconnect/loader/ScriptCacheActors.h +++ b/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& scripts); private: bool mWantCacheData = false; diff --git a/js/xpconnect/loader/ScriptPreloader-inl.h b/js/xpconnect/loader/ScriptPreloader-inl.h index d1b7a3f348a3..6230775ca32b 100644 --- a/js/xpconnect/loader/ScriptPreloader-inl.h +++ b/js/xpconnect/loader/ScriptPreloader-inl.h @@ -208,105 +208,6 @@ public: size_t cursor_ = 0; }; - -template 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(hash)) { -// if (elem->IsDead()) { -// elem.Remove(); -// } -// } -template -class HashElemIter -{ - using Iterator = typename T::Iterator; - using ElemType = typename T::UserDataType; - - T& hash_; - Matcher* matcher_; - Maybe iter_; - -public: - explicit HashElemIter(T& hash, Matcher* matcher = nullptr) - : hash_(hash), matcher_(matcher) - { - iter_.emplace(Move(hash.Iter())); - } - - class Elem - { - friend class HashElemIter; - - HashElemIter& 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 -HashElemIter IterHash(T& hash, Matcher* matcher = nullptr) -{ - return HashElemIter(hash, matcher); -} - -template -bool -Find(T&& iter, F&& match) -{ - for (auto& elem : iter) { - if (match(elem)) { - return true; - } - } - return false; -} - }; // namespace loader }; // namespace mozilla diff --git a/js/xpconnect/loader/ScriptPreloader.cpp b/js/xpconnect/loader/ScriptPreloader.cpp index 896c0f784e13..4ecd0e598960 100644 --- a/js/xpconnect/loader/ScriptPreloader.cpp +++ b/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(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(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& 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([&] () { - mScripts.Clear(); - }); + AutoCleanLinkedList scripts; Range 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 : IterHash(mScripts, Match())) { + for (auto script : mRestoredScripts) { // 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) { - return (script->mStatus == ScriptStatus::Restored || - !script->HasRange() || script->HasArray()); - }); - - if (!found) { - mSaveComplete = true; - return; + if (mRestoredScripts.isEmpty()) { + // Check for any new scripts that we need to save. If there aren't + // 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; + for (auto script : mSavedScripts) { + if (!script->HasRange() || script->HasArray()) { + found = true; + break; + } + } + if (!found) { + mSaveComplete = true; + return; + } } AutoSafeJSAPI jsapi; - for (auto& script : IterHash(mScripts, Match())) { + 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 scripts; - for (auto& script : IterHash(mScripts, Match())) { + for (auto script : mSavedScripts) { scripts.AppendElement(script); } @@ -664,6 +690,17 @@ ScriptPreloader::Run() return NS_OK; } +/* static */ ScriptPreloader::CachedScript* +ScriptPreloader::FindScript(LinkedList& 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) { - script->mStatus = ScriptStatus::Saved; + if (restored) { + 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&& 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) { - script->mStatus = ScriptStatus::Saved; + if (restored) { + 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); } diff --git a/js/xpconnect/loader/ScriptPreloader.h b/js/xpconnect/loader/ScriptPreloader.h index 95b038caebaa..c2c2d26b4781 100644 --- a/js/xpconnect/loader/ScriptPreloader.h +++ b/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 - 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 { 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 - { - 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 mScript; @@ -320,13 +301,6 @@ private: MaybeOneOf> mXDRData; }; - template - static Matcher* 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& scripts); // Opens the cache file for reading. Result OpenCache(); @@ -366,6 +341,8 @@ private: Result, nsresult> GetCacheFile(const nsAString& suffix); + static CachedScript* FindScript(LinkedList& 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; - - template - static size_t SizeOfHashEntries(ScriptHash& scripts, mozilla::MallocSizeOf mallocSizeOf) + template + static size_t SizeOfLinkedList(LinkedList& list, mozilla::MallocSizeOf mallocSizeOf) { size_t size = 0; - for (auto elem : IterHash(scripts, Match())) { + 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 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 mRestoredScripts; + + nsDataHashtable mScripts; // True after we've shown the first window, and are no longer adding new // scripts to the cache.