From a221ac674fe5c8574b0e4aa64e066845c422f092 Mon Sep 17 00:00:00 2001 From: Henry Chang Date: Thu, 15 Jun 2017 15:51:29 +0800 Subject: [PATCH] Bug 1355746 - Part 2. Polish IdleTaskRunner and reuse it for background parsing. r=smaug This patch is mainly to make IdleTaskRunner reusable by nsHtml5TreeOpExecutor. The only necessary work to that purpose is to remove the dependency of sShuttingDown, which was a static variable in nsJSEnvironment.cpp. The idea is to have a "ShouldCancel" as a callback for the consumer to return sShuttingDown. In addition to sShuttingDown, we use std::function as the runner main callback type. MozReview-Commit-ID: FT2X1unSvPS --HG-- extra : rebase_source : cfd99aba19f014327875683f5ea85d183c8af674 extra : intermediate-source : 99af874c7b1d278057194894d406474b8af07349 extra : source : 792359c898f68241e1373820ea8fd3ba18b09994 --- dom/base/nsJSEnvironment.cpp | 30 ++++++++--------- parser/html/nsHtml5TreeOpExecutor.cpp | 39 ++++++++++++---------- xpcom/threads/IdleTaskRunner.cpp | 48 ++++++++++++++++++--------- xpcom/threads/IdleTaskRunner.h | 32 ++++++++++++------ 4 files changed, 92 insertions(+), 57 deletions(-) diff --git a/dom/base/nsJSEnvironment.cpp b/dom/base/nsJSEnvironment.cpp index 36dc5d664614..0d8daf14bc18 100644 --- a/dom/base/nsJSEnvironment.cpp +++ b/dom/base/nsJSEnvironment.cpp @@ -1602,7 +1602,7 @@ nsJSContext::GetMaxCCSliceTimeSinceClear() } static bool -ICCRunnerFired(TimeStamp aDeadline, void* aData) +ICCRunnerFired(TimeStamp aDeadline) { if (sDidShutdown) { return false; @@ -1643,8 +1643,8 @@ nsJSContext::BeginCycleCollectionCallback() // Create an ICC timer even if ICC is globally disabled, because we could be manually triggering // an incremental collection, and we want to be sure to finish it. - sICCRunner = CollectorRunner::Create(ICCRunnerFired, kICCIntersliceDelay, - kIdleICCSliceBudget, true); + sICCRunner = IdleTaskRunner::Create(ICCRunnerFired, kICCIntersliceDelay, + kIdleICCSliceBudget, true, []{ return sShuttingDown; }); } static_assert(NS_GC_DELAY > kMaxICCDuration, "A max duration ICC shouldn't reduce GC delay to 0"); @@ -1856,11 +1856,9 @@ GCTimerFired(nsITimer *aTimer, void *aClosure) { nsJSContext::KillGCTimer(); // Now start the actual GC after initial timer has fired. - sInterSliceGCRunner = CollectorRunner::Create(InterSliceGCRunnerFired, - NS_INTERSLICE_GC_DELAY, - sActiveIntersliceGCBudget, - false, - aClosure); + sInterSliceGCRunner = IdleTaskRunner::Create([aClosure](TimeStamp aDeadline) { + return InterSliceGCRunnerFired(aDeadline, aClosure); + }, NS_INTERSLICE_GC_DELAY, sActiveIntersliceGCBudget, false, []{ return sShuttingDown; }); } // static @@ -1884,7 +1882,7 @@ ShouldTriggerCC(uint32_t aSuspected) } static bool -CCRunnerFired(TimeStamp aDeadline, void* aData) +CCRunnerFired(TimeStamp aDeadline) { if (sDidShutdown) { return false; @@ -2036,13 +2034,13 @@ nsJSContext::RunNextCollectorTimer() if (sCCRunner) { if (ReadyToTriggerExpensiveCollectorTimer()) { - CCRunnerFired(TimeStamp(), nullptr); + CCRunnerFired(TimeStamp()); } return; } if (sICCRunner) { - ICCRunnerFired(TimeStamp(), nullptr); + ICCRunnerFired(TimeStamp()); return; } } @@ -2149,8 +2147,9 @@ nsJSContext::MaybePokeCC() nsCycleCollector_dispatchDeferredDeletion(); sCCRunner = - CollectorRunner::Create(CCRunnerFired, NS_CC_SKIPPABLE_DELAY, - kForgetSkippableSliceDuration, true); + IdleTaskRunner::Create(CCRunnerFired, NS_CC_SKIPPABLE_DELAY, + kForgetSkippableSliceDuration, true, + []{ return sShuttingDown; }); } } @@ -2330,8 +2329,9 @@ DOMGCSliceCallback(JSContext* aCx, JS::GCProgress aProgress, const JS::GCDescrip nsJSContext::KillInterSliceGCRunner(); if (!sShuttingDown && !aDesc.isComplete_) { sInterSliceGCRunner = - CollectorRunner::Create(InterSliceGCRunnerFired, NS_INTERSLICE_GC_DELAY, - sActiveIntersliceGCBudget, false); + IdleTaskRunner::Create([](TimeStamp aDeadline) { + return InterSliceGCRunnerFired(aDeadline, nullptr); + }, NS_INTERSLICE_GC_DELAY, sActiveIntersliceGCBudget, false, []{ return sShuttingDown; }); } if (ShouldTriggerCC(nsCycleCollector_suspectedCount())) { diff --git a/parser/html/nsHtml5TreeOpExecutor.cpp b/parser/html/nsHtml5TreeOpExecutor.cpp index 54e8c2eead2b..860196b31168 100644 --- a/parser/html/nsHtml5TreeOpExecutor.cpp +++ b/parser/html/nsHtml5TreeOpExecutor.cpp @@ -33,6 +33,7 @@ #include "nsIHTMLDocument.h" #include "nsIViewSourceChannel.h" #include "xpcpublic.h" +#include "mozilla/IdleTaskRunner.h" using namespace mozilla; @@ -61,7 +62,7 @@ class nsHtml5ExecutorReflusher : public Runnable }; static mozilla::LinkedList* gBackgroundFlushList = nullptr; -static nsITimer* gFlushTimer = nullptr; +StaticRefPtr gBackgroundFlushRunner; nsHtml5TreeOpExecutor::nsHtml5TreeOpExecutor() : nsHtml5DocumentBuilder(false) @@ -85,9 +86,9 @@ nsHtml5TreeOpExecutor::~nsHtml5TreeOpExecutor() if (gBackgroundFlushList->isEmpty()) { delete gBackgroundFlushList; gBackgroundFlushList = nullptr; - if (gFlushTimer) { - gFlushTimer->Cancel(); - NS_RELEASE(gFlushTimer); + if (gBackgroundFlushRunner) { + gBackgroundFlushRunner->Cancel(); + gBackgroundFlushRunner = nullptr; } } } @@ -245,9 +246,10 @@ nsHtml5TreeOpExecutor::MarkAsBroken(nsresult aReason) return aReason; } -void -FlushTimerCallback(nsITimer* aTimer, void* aClosure) +static bool +BackgroundFlushCallback(TimeStamp /*aDeadline*/) { + NS_WARNING("Executing BackgroundFlushCallback()...."); RefPtr ex = gBackgroundFlushList->popFirst(); if (ex) { ex->RunFlushLoop(); @@ -255,9 +257,11 @@ FlushTimerCallback(nsITimer* aTimer, void* aClosure) if (gBackgroundFlushList && gBackgroundFlushList->isEmpty()) { delete gBackgroundFlushList; gBackgroundFlushList = nullptr; - gFlushTimer->Cancel(); - NS_RELEASE(gFlushTimer); + gBackgroundFlushRunner->Cancel(); + gBackgroundFlushRunner = nullptr; + return true; } + return true; } void @@ -277,16 +281,17 @@ nsHtml5TreeOpExecutor::ContinueInterruptedParsingAsync() if (!isInList()) { gBackgroundFlushList->insertBack(this); } - if (!gFlushTimer) { - nsCOMPtr t = do_CreateInstance("@mozilla.org/timer;1"); - t.swap(gFlushTimer); - // The timer value 50 should not hopefully slow down background pages too - // much, yet lets event loop to process enough between ticks. - // See bug 734015. - gFlushTimer->InitWithNamedFuncCallback(FlushTimerCallback, nullptr, - 50, nsITimer::TYPE_REPEATING_SLACK, - "FlushTimerCallback"); + if (gBackgroundFlushRunner) { + NS_WARNING("We've already scheduled a task for background list flush."); + return; } + // Now we set up a repetitive idle scheduler for flushing background list. + gBackgroundFlushRunner = + IdleTaskRunner::Create(&BackgroundFlushCallback, + 250, // The hard deadline: 250ms. + nsContentSink::sInteractiveTime / 1000, // Required budget. + true, // repeating + []{ return false; }); // MaybeContinueProcess } } diff --git a/xpcom/threads/IdleTaskRunner.cpp b/xpcom/threads/IdleTaskRunner.cpp index 2c5b4323fe6d..824e597ce0be 100644 --- a/xpcom/threads/IdleTaskRunner.cpp +++ b/xpcom/threads/IdleTaskRunner.cpp @@ -10,25 +10,28 @@ namespace mozilla { already_AddRefed -IdleTaskRunner::Create(IdleTaskRunnerCallback aCallback, uint32_t aDelay, - int64_t aBudget, bool aRepeating, void* aData) +IdleTaskRunner::Create(const CallbackType& aCallback, uint32_t aDelay, + int64_t aBudget, bool aRepeating, + const MayContinueProcessingCallbackType& aMaybeContinueProcessing) { - if (sShuttingDown) { + if (aMaybeContinueProcessing && aMaybeContinueProcessing()) { return nullptr; } RefPtr runner = - new IdleTaskRunner(aCallback, aDelay, aBudget, aRepeating, aData); + new IdleTaskRunner(aCallback, aDelay, aBudget, aRepeating, aMaybeContinueProcessing); runner->Schedule(false); // Initial scheduling shouldn't use idle dispatch. return runner.forget(); } -IdleTaskRunner::IdleTaskRunner(IdleTaskRunnerCallback aCallback, +IdleTaskRunner::IdleTaskRunner(const CallbackType& aCallback, uint32_t aDelay, int64_t aBudget, - bool aRepeating, void* aData) + bool aRepeating, + const MayContinueProcessingCallbackType& aMaybeContinueProcessing) : mCallback(aCallback), mDelay(aDelay) , mBudget(TimeDuration::FromMilliseconds(aBudget)) - , mRepeating(aRepeating), mTimerActive(false), mData(aData) + , mRepeating(aRepeating), mTimerActive(false) + , mMaybeContinueProcessing(aMaybeContinueProcessing) { } @@ -40,18 +43,25 @@ IdleTaskRunner::Run() } // Deadline is null when called from timer. + TimeStamp now = TimeStamp::Now(); bool deadLineWasNull = mDeadline.IsNull(); bool didRun = false; - if (deadLineWasNull || ((TimeStamp::Now() + mBudget) < mDeadline)) { + bool allowIdleDispatch = false; + if (deadLineWasNull || ((now + mBudget) < mDeadline)) { CancelTimer(); - didRun = mCallback(mDeadline, mData); - } - - if (mCallback && (mRepeating || !didRun)) { + didRun = mCallback(mDeadline); // If we didn't do meaningful work, don't schedule using immediate // idle dispatch, since that could lead to a loop until the idle // period ends. - Schedule(didRun); + allowIdleDispatch = didRun; + } else if (now >= mDeadline) { + allowIdleDispatch = true; + } + + if (mCallback && (mRepeating || !didRun)) { + Schedule(allowIdleDispatch); + } else { + mCallback = nullptr; } return NS_OK; @@ -114,11 +124,12 @@ IdleTaskRunner::Schedule(bool aAllowIdleDispatch) return; } - if (sShuttingDown) { + if (mMaybeContinueProcessing && mMaybeContinueProcessing()) { Cancel(); return; } + TimeStamp deadline = mDeadline; mDeadline = TimeStamp(); TimeStamp now = TimeStamp::Now(); TimeStamp hint = nsRefreshDriver::GetIdleDeadlineHint(now); @@ -145,7 +156,14 @@ IdleTaskRunner::Schedule(bool aAllowIdleDispatch) // We weren't allowed to do idle dispatch immediately, do it after a // short timeout. - mScheduleTimer->InitWithFuncCallback(ScheduleTimedOut, this, 16, + uint32_t lateScheduleDelayMs; + if (deadline.IsNull()) { + lateScheduleDelayMs = 16; + } else { + lateScheduleDelayMs = + (uint32_t)((deadline - now).ToMilliseconds()); + } + mScheduleTimer->InitWithFuncCallback(ScheduleTimedOut, this, lateScheduleDelayMs, nsITimer::TYPE_ONE_SHOT_LOW_PRIORITY); } } diff --git a/xpcom/threads/IdleTaskRunner.h b/xpcom/threads/IdleTaskRunner.h index 604aa99e8c07..db235d923cc0 100644 --- a/xpcom/threads/IdleTaskRunner.h +++ b/xpcom/threads/IdleTaskRunner.h @@ -11,16 +11,27 @@ namespace mozilla { -// Return true if some meaningful work was done. -typedef bool (*IdleTaskRunnerCallback) (TimeStamp aDeadline, void* aData); - -// Repeating callback runner for CC and GC. +// A general purpose repeating callback runner (it can be configured +// to a one-time runner, too.) If it is running repeatedly, +// one has to either explicitly Cancel() the runner or have +// MayContinueProcessing() callback return false to completely remove +// the runner. class IdleTaskRunner final : public IdleRunnable { +public: + // Return true if some meaningful work was done. + using CallbackType = std::function; + + // A callback for "continue process" decision. Return false to + // stop processing. This can be an alternative to Cancel() or + // work together in different way. + using MayContinueProcessingCallbackType = std::function; + public: static already_AddRefed - Create(IdleTaskRunnerCallback aCallback, uint32_t aDelay, - int64_t aBudget, bool aRepeating, void* aData = nullptr); + Create(const CallbackType& aCallback, uint32_t aDelay, + int64_t aBudget, bool aRepeating, + const MayContinueProcessingCallbackType& aMaybeContinueProcessing); NS_IMETHOD Run() override; @@ -31,22 +42,23 @@ public: void Schedule(bool aAllowIdleDispatch); private: - explicit IdleTaskRunner(IdleTaskRunnerCallback aCallback, + explicit IdleTaskRunner(const CallbackType& aCallback, uint32_t aDelay, int64_t aBudget, - bool aRepeating, void* aData); + bool aRepeating, + const MayContinueProcessingCallbackType& aMaybeContinueProcessing); ~IdleTaskRunner(); void CancelTimer(); nsCOMPtr mTimer; nsCOMPtr mScheduleTimer; nsCOMPtr mTarget; - IdleTaskRunnerCallback mCallback; + CallbackType mCallback; uint32_t mDelay; TimeStamp mDeadline; TimeDuration mBudget; bool mRepeating; bool mTimerActive; - void* mData; + MayContinueProcessingCallbackType mMaybeContinueProcessing; }; } // end of unnamed namespace.