From 90032b07986d79090c2ab2f7df582cc3440387fa Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 2 Nov 2017 17:11:51 +1100 Subject: [PATCH 01/13] Bug 1413811 (part 1) - Avoid PrefCallback for pref callbacks registered with Preferences::RegisterCallback(). r=glandium. Pref callbacks registered via Preferences::Add*VarCache() are wrapped in a ValueObserver -- which groups all callback requests that have the same prefname and callback function -- and the ValueObserver is put into gObserverTable. This is reasonable. Observers registered via nsPrefBranch::Add{Weak,Strong}Observer() are wrapped in a PrefCallback, and the PrefCallback is put into sRootBranch->mObservers. This is also reasonable. Pref callbacks registered via Preferences::RegisterCallback() are conceptually similar to those registered via Preferences::Add*VarCache(). However, they are implemented by using *both* of the above mechanisms: they are wrapped in a ValueObserver which is put into gObserverTable, *and* that ValueObserver is then wrapped in a PrefCallback which is put into sRootBranch->mObservers. Using both mechanisms isn't necessary, so this patch removes the PrefCallback/mObservers part. This makes Preferences::RegisterCallback() work in much the same way as Preferences::Add*VarCache(). Specifically: - Preferences::RegisterCallback() now calls PREF_RegisterCallback() instead of Preferences::AddStrongObserver(). This makes it more similar to RegisterPriorityCallback(). - Preferences::UnregisterCallback() now explicitly calls PREF_UnregisterCallback() instead of Preferences::RemoveObserver (which previously happened via ~ValueObserver() when the ValueObserver was removed from gObserverTable and its refcount dropped to zerod). MozReview-Commit-ID: 1tEQNeYrBUU --HG-- extra : rebase_source : 431a42402397a172d562b81b8d46418503bb8dfe --- modules/libpref/Preferences.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/modules/libpref/Preferences.cpp b/modules/libpref/Preferences.cpp index b203a2722b6a..e9499333c549 100644 --- a/modules/libpref/Preferences.cpp +++ b/modules/libpref/Preferences.cpp @@ -3263,7 +3263,7 @@ class ValueObserver final : public nsIObserver , public ValueObserverHashKey { - ~ValueObserver() { Preferences::RemoveObserver(this, mPrefName.get()); } + ~ValueObserver() = default; public: NS_DECL_ISUPPORTS @@ -4993,9 +4993,10 @@ Preferences::RegisterCallback(PrefChangedFunc aCallback, observer = new ValueObserver(aPref, aCallback, aMatchKind); observer->AppendClosure(aClosure); - nsresult rv = AddStrongObserver(observer, aPref); - NS_ENSURE_SUCCESS(rv, rv); - + PREF_RegisterCallback(aPref, + NotifyObserver, + static_cast(observer), + /* isPriority */ false); gObserverTable->Put(observer, observer); return NS_OK; } @@ -5037,6 +5038,9 @@ Preferences::UnregisterCallback(PrefChangedFunc aCallback, observer->RemoveClosure(aClosure); if (observer->HasNoClosures()) { // Delete the callback since its list of closures is empty. + MOZ_ALWAYS_SUCCEEDS( + PREF_UnregisterCallback(aPref, NotifyObserver, observer)); + gObserverTable->Remove(observer); } return NS_OK; From d8ca9e91d7b278b96e77fd6b99e2382412c72423 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 2 Nov 2017 17:13:47 +1100 Subject: [PATCH 02/13] Bug 1413811 (part 2) - Factor out similarities between RegisterPriorityCallback() and Preferences::RegisterCallback(). r=glandium. MozReview-Commit-ID: 8K3RNjZTSc3 --HG-- extra : rebase_source : 1bd0aacc120248f024e2e428af40087bba3f6848 --- modules/libpref/Preferences.cpp | 51 +++++++++++++++------------------ 1 file changed, 23 insertions(+), 28 deletions(-) diff --git a/modules/libpref/Preferences.cpp b/modules/libpref/Preferences.cpp index e9499333c549..24a6593e9145 100644 --- a/modules/libpref/Preferences.cpp +++ b/modules/libpref/Preferences.cpp @@ -4681,8 +4681,7 @@ pref_InitInitialObjects() if (!strcmp(NS_STRINGIFY(MOZ_UPDATE_CHANNEL), "nightly") || !strcmp(NS_STRINGIFY(MOZ_UPDATE_CHANNEL), "aurora") || - !strcmp(NS_STRINGIFY(MOZ_UPDATE_CHANNEL), "beta") || - developerBuild) { + !strcmp(NS_STRINGIFY(MOZ_UPDATE_CHANNEL), "beta") || developerBuild) { PREF_SetBoolPref(kTelemetryPref, true, true); } else { PREF_SetBoolPref(kTelemetryPref, false, true); @@ -4951,13 +4950,13 @@ NotifyObserver(const char* aPref, void* aClosure) } static void -RegisterPriorityCallback(PrefChangedFunc aCallback, - const char* aPref, - void* aClosure) +RegisterCallbackHelper(PrefChangedFunc aCallback, + const char* aPref, + void* aClosure, + Preferences::MatchKind aMatchKind, + bool aIsPriority) { - MOZ_ASSERT(Preferences::IsServiceAvailable()); - - ValueObserverHashKey hashKey(aPref, aCallback, Preferences::ExactMatch); + ValueObserverHashKey hashKey(aPref, aCallback, aMatchKind); RefPtr observer; gObserverTable->Get(&hashKey, getter_AddRefs(observer)); if (observer) { @@ -4965,15 +4964,24 @@ RegisterPriorityCallback(PrefChangedFunc aCallback, return; } - observer = new ValueObserver(aPref, aCallback, Preferences::ExactMatch); + observer = new ValueObserver(aPref, aCallback, aMatchKind); observer->AppendClosure(aClosure); - PREF_RegisterCallback(aPref, - NotifyObserver, - static_cast(observer), - /* isPriority */ true); + PREF_RegisterCallback( + aPref, NotifyObserver, static_cast(observer), aIsPriority); gObserverTable->Put(observer, observer); } +static void +RegisterPriorityCallback(PrefChangedFunc aCallback, + const char* aPref, + void* aClosure) +{ + MOZ_ASSERT(Preferences::IsServiceAvailable()); + + RegisterCallbackHelper( + aCallback, aPref, aClosure, Preferences::ExactMatch, /* isPriority */ true); +} + /* static */ nsresult Preferences::RegisterCallback(PrefChangedFunc aCallback, const char* aPref, @@ -4983,21 +4991,8 @@ Preferences::RegisterCallback(PrefChangedFunc aCallback, MOZ_ASSERT(aCallback); NS_ENSURE_TRUE(InitStaticMembers(), NS_ERROR_NOT_AVAILABLE); - ValueObserverHashKey hashKey(aPref, aCallback, aMatchKind); - RefPtr observer; - gObserverTable->Get(&hashKey, getter_AddRefs(observer)); - if (observer) { - observer->AppendClosure(aClosure); - return NS_OK; - } - - observer = new ValueObserver(aPref, aCallback, aMatchKind); - observer->AppendClosure(aClosure); - PREF_RegisterCallback(aPref, - NotifyObserver, - static_cast(observer), - /* isPriority */ false); - gObserverTable->Put(observer, observer); + RegisterCallbackHelper( + aCallback, aPref, aClosure, aMatchKind, /* isPriority */ false); return NS_OK; } From ee320f219eb5ae071040bbaf32bc42b2ec0bdeff Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 2 Nov 2017 17:20:36 +1100 Subject: [PATCH 03/13] Bug 1413811 (part 3) - Rename RegisterPriorityCallback(). r=glandium. MozReview-Commit-ID: 5Ov1cHgfB5Y --HG-- extra : rebase_source : 767ae8c0390f68555e334945cb5479479a52e1ef --- modules/libpref/Preferences.cpp | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/modules/libpref/Preferences.cpp b/modules/libpref/Preferences.cpp index 24a6593e9145..1c8520d04c10 100644 --- a/modules/libpref/Preferences.cpp +++ b/modules/libpref/Preferences.cpp @@ -4971,8 +4971,11 @@ RegisterCallbackHelper(PrefChangedFunc aCallback, gObserverTable->Put(observer, observer); } +// RegisterVarCacheCallback uses high priority callbacks to ensure that cache +// observers are called prior to ordinary pref observers. Doing this ensures +// that ordinary observers will never get stale values from cache variables. static void -RegisterPriorityCallback(PrefChangedFunc aCallback, +RegisterVarCacheCallback(PrefChangedFunc aCallback, const char* aPref, void* aClosure) { @@ -5041,10 +5044,6 @@ Preferences::UnregisterCallback(PrefChangedFunc aCallback, return NS_OK; } -// We insert cache observers using RegisterPriorityCallback to ensure they are -// called prior to ordinary pref observers. Doing this ensures that ordinary -// observers will never get stale values from cache variables. - static void BoolVarChanged(const char* aPref, void* aClosure) { @@ -5075,7 +5074,7 @@ Preferences::AddBoolVarCache(bool* aCache, const char* aPref, bool aDefault) data->mCacheLocation = aCache; data->mDefaultValueBool = aDefault; CacheDataAppendElement(data); - RegisterPriorityCallback(BoolVarChanged, aPref, data); + RegisterVarCacheCallback(BoolVarChanged, aPref, data); return NS_OK; } @@ -5102,7 +5101,7 @@ Preferences::AddIntVarCache(int32_t* aCache, data->mCacheLocation = aCache; data->mDefaultValueInt = aDefault; CacheDataAppendElement(data); - RegisterPriorityCallback(IntVarChanged, aPref, data); + RegisterVarCacheCallback(IntVarChanged, aPref, data); return NS_OK; } @@ -5129,7 +5128,7 @@ Preferences::AddUintVarCache(uint32_t* aCache, data->mCacheLocation = aCache; data->mDefaultValueUint = aDefault; CacheDataAppendElement(data); - RegisterPriorityCallback(UintVarChanged, aPref, data); + RegisterVarCacheCallback(UintVarChanged, aPref, data); return NS_OK; } @@ -5158,7 +5157,7 @@ Preferences::AddAtomicUintVarCache(Atomic* aCache, data->mCacheLocation = aCache; data->mDefaultValueUint = aDefault; CacheDataAppendElement(data); - RegisterPriorityCallback(AtomicUintVarChanged, aPref, data); + RegisterVarCacheCallback(AtomicUintVarChanged, aPref, data); return NS_OK; } @@ -5191,7 +5190,7 @@ Preferences::AddFloatVarCache(float* aCache, const char* aPref, float aDefault) data->mCacheLocation = aCache; data->mDefaultValueFloat = aDefault; CacheDataAppendElement(data); - RegisterPriorityCallback(FloatVarChanged, aPref, data); + RegisterVarCacheCallback(FloatVarChanged, aPref, data); return NS_OK; } From e0ae5de7ef84b2f63d9c00d36e15d1d2d3f238d9 Mon Sep 17 00:00:00 2001 From: Junior Hsu Date: Sun, 5 Nov 2017 17:48:36 -0500 Subject: [PATCH 04/13] Bug 1412218 - Add telemetry probe for how many users with legacy cookie files. r=jdm, data-r=francois --- netwerk/cookie/nsCookieService.cpp | 3 +++ toolkit/components/telemetry/Histograms.json | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/netwerk/cookie/nsCookieService.cpp b/netwerk/cookie/nsCookieService.cpp index 0502905355a7..5142ad24e5c9 100644 --- a/netwerk/cookie/nsCookieService.cpp +++ b/netwerk/cookie/nsCookieService.cpp @@ -1338,6 +1338,8 @@ nsCookieService::TryInitDB(bool aRecreateDB) // No more upgrades. Update the schema version. rv = mDefaultDBState->syncConn->SetSchemaVersion(COOKIES_SCHEMA_VERSION); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); + + Telemetry::Accumulate(Telemetry::MOZ_SQLITE_COOKIES_OLD_SCHEMA, dbSchemaVersion); MOZ_FALLTHROUGH; case COOKIES_SCHEMA_VERSION: @@ -1429,6 +1431,7 @@ nsCookieService::TryInitDB(bool aRecreateDB) gCookieService->ImportCookies(oldCookieFile); oldCookieFile->Remove(false); gCookieService->mDBState = initialState; + Telemetry::Accumulate(Telemetry::MOZ_SQLITE_COOKIES_OLD_SCHEMA, 0); }); NS_DispatchToMainThread(runnable); diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index f0e0db8a4b53..0cec3d613fba 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -4426,6 +4426,15 @@ "n_buckets": 10, "description": "Time spent on SQLite read() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" }, + "MOZ_SQLITE_COOKIES_OLD_SCHEMA": { + "record_in_processes": ["main"], + "expires_in_version": "62", + "kind": "enumerated", + "n_values": 10, + "bug_numbers": [1412218], + "alert_emails": ["necko@mozilla.com", "junior@mozilla.com"], + "description": "Old schema version of the cookie database. 0 for having legacy cookies.txt." + }, "MOZ_SQLITE_COOKIES_BLOCK_MAIN_THREAD_MS": { "record_in_processes": ["main"], "expires_in_version": "never", From dcd73319bd9ef96b7406789fe4063538c859d1c4 Mon Sep 17 00:00:00 2001 From: Bob Clary Date: Sun, 5 Nov 2017 16:58:48 -0800 Subject: [PATCH 05/13] Bug 1408241 - disable Mdm1 dom/media/test/test_mediarecorder_record_addtracked_stream, r=jmaher. --- dom/media/test/mochitest.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/dom/media/test/mochitest.ini b/dom/media/test/mochitest.ini index a505b97e1864..088f7446984f 100644 --- a/dom/media/test/mochitest.ini +++ b/dom/media/test/mochitest.ini @@ -849,6 +849,7 @@ tags=msg skip-if = android_version == '17' # android(bug 1232305) tags=msg [test_mediarecorder_record_addtracked_stream.html] +skip-if = toolkit == 'android' # Bug 1408241 tags=msg capturestream [test_mediarecorder_record_audiocontext.html] skip-if = android_version == '17' # android(bug 1232305) From 1d8ba0f54083d0f50addee09d87ee7066c0939b9 Mon Sep 17 00:00:00 2001 From: Bob Clary Date: Sun, 5 Nov 2017 16:59:19 -0800 Subject: [PATCH 06/13] Bug 1365451 - disable Cdm1 dom/media/test/crashtests/691096-1.html, r=jmaher. --- dom/media/test/crashtests/crashtests.list | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dom/media/test/crashtests/crashtests.list b/dom/media/test/crashtests/crashtests.list index 5f2d363bb92f..3fde1fdb2387 100644 --- a/dom/media/test/crashtests/crashtests.list +++ b/dom/media/test/crashtests/crashtests.list @@ -108,6 +108,6 @@ load oscillator-ended-1.html load oscillator-ended-2.html skip-if(Android&&AndroidVersion=='22') load video-replay-after-audio-end.html # bug 1315125, bug 1358876 # This needs to run at the end to avoid leaking busted state into other tests. -load 691096-1.html +skip-if(Android) load 691096-1.html # Bug 1365451 load 1236639.html test-pref(media.navigator.permission.disabled,true) load 1388372.html From 76895c46c88ddf20c698b7cc7f8de360b8cc8c6c Mon Sep 17 00:00:00 2001 From: Paul Bone Date: Wed, 18 Oct 2017 09:17:04 +1100 Subject: [PATCH 07/13] Bug 1298018 - Part 1: Clarify emptyChunks_ comment. r=jonco --- js/src/gc/GCRuntime.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/js/src/gc/GCRuntime.h b/js/src/gc/GCRuntime.h index 4d19f5269228..e9f404f84e11 100644 --- a/js/src/gc/GCRuntime.h +++ b/js/src/gc/GCRuntime.h @@ -1210,10 +1210,11 @@ class GCRuntime AtomMarkingRuntime atomMarking; private: - // When empty, chunks reside in the emptyChunks pool and are re-used as - // needed or eventually expired if not re-used. The emptyChunks pool gets - // refilled from the background allocation task heuristically so that empty - // chunks should always available for immediate allocation without syscalls. + // When chunks are empty, they reside in the emptyChunks pool and are + // re-used as needed or eventually expired if not re-used. The emptyChunks + // pool gets refilled from the background allocation task heuristically so + // that empty chunks should always be available for immediate allocation + // without syscalls. GCLockData emptyChunks_; // Chunks which have had some, but not all, of their arenas allocated live From 033bd6efd1e74c2b26184838dcf82fbf74510b89 Mon Sep 17 00:00:00 2001 From: Paul Bone Date: Mon, 16 Oct 2017 21:29:07 +1100 Subject: [PATCH 08/13] Bug 1298018 - Part 2: Remove some unused methods. r=jonco --- js/src/gc/Nursery.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/js/src/gc/Nursery.h b/js/src/gc/Nursery.h index 0af23e62254d..2d785be0cb4d 100644 --- a/js/src/gc/Nursery.h +++ b/js/src/gc/Nursery.h @@ -145,7 +145,6 @@ class Nursery unsigned numChunks() const { return chunks_.length(); } bool exists() const { return maxChunks() != 0; } - size_t nurserySize() const { return maxChunks() << ChunkShift; } void enable(); void disable(); @@ -419,8 +418,6 @@ class Nursery Canary* lastCanary_; #endif - NurseryChunk* allocChunk(); - NurseryChunk& chunk(unsigned index) const { return *chunks_[index]; } From ecd852a84c1f46da2ea44d966b53b4dff0be02e9 Mon Sep 17 00:00:00 2001 From: Paul Bone Date: Mon, 30 Oct 2017 13:17:11 +1100 Subject: [PATCH 09/13] Bug 1298018 - Part 3: Replace updateNumChunks(). r=jonco This method both shrinks and grows the nursery. This change replaces it with specialised code to either shrink or grow the nursery. --- js/src/gc/Nursery.cpp | 154 +++++++++++++++++++++++++----------------- js/src/gc/Nursery.h | 17 +++-- 2 files changed, 103 insertions(+), 68 deletions(-) diff --git a/js/src/gc/Nursery.cpp b/js/src/gc/Nursery.cpp index 1ea06efde9b1..9213d2c28a28 100644 --- a/js/src/gc/Nursery.cpp +++ b/js/src/gc/Nursery.cpp @@ -146,8 +146,7 @@ js::Nursery::init(uint32_t maxNurseryBytes, AutoLockGCBgAlloc& lock) if (maxNurseryChunks_ == 0) return true; - updateNumChunksLocked(1, lock); - if (numChunks() == 0) + if (!allocateFirstChunk(lock)) return false; setCurrentChunk(0); @@ -195,9 +194,11 @@ js::Nursery::enable() if (isEnabled() || !maxChunks()) return; - updateNumChunks(1); - if (numChunks() == 0) - return; + { + AutoLockGCBgAlloc lock(runtime()); + if (!allocateFirstChunk(lock)) + return; + } setCurrentChunk(0); setStartPosition(); @@ -215,7 +216,8 @@ js::Nursery::disable() MOZ_ASSERT(isEmpty()); if (!isEnabled()) return; - updateNumChunks(0); + + freeChunksFrom(0); currentEnd_ = 0; runtime()->gc.storeBuffer().disable(); } @@ -237,7 +239,7 @@ js::Nursery::isEmpty() const void js::Nursery::enterZealMode() { if (isEnabled()) - updateNumChunks(maxNurseryChunks_); + growAllocableSpace(maxChunks()); } void @@ -1001,93 +1003,119 @@ js::Nursery::maybeResizeNursery(JS::gcreason::Reason reason) if (newMaxNurseryChunks != maxNurseryChunks_) { maxNurseryChunks_ = newMaxNurseryChunks; /* The configured maximum nursery size is changing */ - const int extraChunks = numChunks() - newMaxNurseryChunks; - if (extraChunks > 0) { + if (numChunks() > newMaxNurseryChunks) { /* We need to shrink the nursery */ - shrinkAllocableSpace(extraChunks); + shrinkAllocableSpace(newMaxNurseryChunks); previousPromotionRate_ = promotionRate; return; } } - if (promotionRate > GrowThreshold) + if (promotionRate > GrowThreshold) { + // The GC nursery is an optimization and so if we fail to allocate + // nursery chunks we do not report an error. growAllocableSpace(); - else if (promotionRate < ShrinkThreshold && previousPromotionRate_ < ShrinkThreshold) - shrinkAllocableSpace(1); + } else if (promotionRate < ShrinkThreshold && previousPromotionRate_ < ShrinkThreshold) { + shrinkAllocableSpace(numChunks() - 1); + } previousPromotionRate_ = promotionRate; } -void +bool js::Nursery::growAllocableSpace() { - updateNumChunks(Min(numChunks() * 2, maxNurseryChunks_)); + return growAllocableSpace(Min(numChunks() * 2, maxChunks())); } -void -js::Nursery::shrinkAllocableSpace(unsigned removeNumChunks) +bool +js::Nursery::growAllocableSpace(unsigned newCount) { -#ifdef JS_GC_ZEAL - if (runtime()->hasZealMode(ZealMode::GenerationalGC)) - return; -#endif - updateNumChunks(Max(numChunks() - removeNumChunks, 1u)); -} - -void -js::Nursery::minimizeAllocableSpace() -{ -#ifdef JS_GC_ZEAL - if (runtime()->hasZealMode(ZealMode::GenerationalGC)) - return; -#endif - updateNumChunks(1); -} - -void -js::Nursery::updateNumChunks(unsigned newCount) -{ - if (numChunks() != newCount) { - AutoLockGCBgAlloc lock(runtime()); - updateNumChunksLocked(newCount, lock); - } -} - -void -js::Nursery::updateNumChunksLocked(unsigned newCount, - AutoLockGCBgAlloc& lock) -{ - // The GC nursery is an optimization and so if we fail to allocate nursery - // chunks we do not report an error. - - MOZ_ASSERT(newCount <= maxChunks()); - unsigned priorCount = numChunks(); - MOZ_ASSERT(priorCount != newCount); - if (newCount < priorCount) { - // Shrink the nursery and free unused chunks. - for (unsigned i = newCount; i < priorCount; i++) - runtime()->gc.recycleChunk(chunk(i).toChunk(runtime()), lock); - chunks_.shrinkTo(newCount); - return; - } + if (newCount == priorCount) + return false; + + MOZ_ASSERT(newCount > priorCount); + MOZ_ASSERT(newCount <= maxChunks()); + MOZ_ASSERT(priorCount >= 1); - // Grow the nursery and allocate new chunks. if (!chunks_.resize(newCount)) - return; + return false; + AutoLockGCBgAlloc lock(runtime()); for (unsigned i = priorCount; i < newCount; i++) { auto newChunk = runtime()->gc.getOrAllocChunk(lock); if (!newChunk) { chunks_.shrinkTo(i); - return; + return false; } chunks_[i] = NurseryChunk::fromChunk(newChunk); chunk(i).poisonAndInit(runtime(), JS_FRESH_NURSERY_PATTERN); } + + return true; +} + +void +js::Nursery::freeChunksFrom(unsigned firstFreeChunk) +{ + MOZ_ASSERT(firstFreeChunk < chunks_.length()); + { + AutoLockGC lock(runtime()); + for (unsigned i = firstFreeChunk; i < chunks_.length(); i++) + runtime()->gc.recycleChunk(chunk(i).toChunk(runtime()), lock); + } + chunks_.shrinkTo(firstFreeChunk); +} + +void +js::Nursery::shrinkAllocableSpace(unsigned newCount) +{ +#ifdef JS_GC_ZEAL + if (runtime()->hasZealMode(ZealMode::GenerationalGC)) + return; +#endif + + // Don't shrink the nursery to zero (use Nursery::disable() instead) and + // don't attempt to shrink it to the same size. + if ((newCount == 0) || (newCount == numChunks())) + return; + + MOZ_ASSERT(newCount < numChunks()); + + freeChunksFrom(newCount); +} + +void +js::Nursery::minimizeAllocableSpace() +{ + shrinkAllocableSpace(1); +} + +bool +js::Nursery::allocateFirstChunk(AutoLockGCBgAlloc& lock) +{ + // This assertion isn't required for correctness, but we do assume this + // is only called to initialize or re-enable the nursery. + MOZ_ASSERT(numChunks() == 0); + + MOZ_ASSERT(maxChunks() > 0); + + if (!chunks_.resize(1)) + return false; + + auto chunk = runtime()->gc.getOrAllocChunk(lock); + if (!chunk) { + chunks_.shrinkTo(0); + return false; + } + + chunks_[0] = NurseryChunk::fromChunk(chunk); + + return true; } bool diff --git a/js/src/gc/Nursery.h b/js/src/gc/Nursery.h index 2d785be0cb4d..7a0a2baa61ed 100644 --- a/js/src/gc/Nursery.h +++ b/js/src/gc/Nursery.h @@ -425,9 +425,11 @@ class Nursery void setCurrentChunk(unsigned chunkno); void setStartPosition(); - void updateNumChunks(unsigned newCount); - void updateNumChunksLocked(unsigned newCount, - AutoLockGCBgAlloc& lock); + /* + * Ensure that the first chunk has been allocated. Callers will probably + * want to call setCurrentChunk(0) next. + */ + MOZ_MUST_USE bool allocateFirstChunk(AutoLockGCBgAlloc& lock); MOZ_ALWAYS_INLINE uintptr_t currentEnd() const; @@ -479,10 +481,15 @@ class Nursery /* Change the allocable space provided by the nursery. */ void maybeResizeNursery(JS::gcreason::Reason reason); - void growAllocableSpace(); - void shrinkAllocableSpace(unsigned removeNumChunks); + bool growAllocableSpace(); + bool growAllocableSpace(unsigned newSize); + void shrinkAllocableSpace(unsigned newCount); void minimizeAllocableSpace(); + // Free the chunks starting at firstFreeChunk until the end of the chunks + // vector. Shrinks the vector but does not update maxChunksAlloc(). + void freeChunksFrom(unsigned firstFreeChunk); + /* Profile recording and printing. */ void maybeClearProfileDurations(); void startProfile(ProfileKey key); From cd89ebc40adf66aa21e3415ac8264808bf4735b6 Mon Sep 17 00:00:00 2001 From: Paul Bone Date: Mon, 30 Oct 2017 14:44:12 +1100 Subject: [PATCH 10/13] Bug 1298018 - Part 4: Lazily allocate nursery chunks. r=jonco --- js/src/gc/Nursery.cpp | 133 ++++++++++++++++++++++++------------------ js/src/gc/Nursery.h | 41 +++++++++---- 2 files changed, 105 insertions(+), 69 deletions(-) diff --git a/js/src/gc/Nursery.cpp b/js/src/gc/Nursery.cpp index 9213d2c28a28..86e83236eca9 100644 --- a/js/src/gc/Nursery.cpp +++ b/js/src/gc/Nursery.cpp @@ -116,7 +116,8 @@ js::Nursery::Nursery(JSRuntime* rt) , currentStartPosition_(0) , currentEnd_(0) , currentChunk_(0) - , maxNurseryChunks_(0) + , maxChunkCount_(0) + , chunkCountLimit_(0) , previousPromotionRate_(0) , profileThreshold_(0) , enableProfiling_(false) @@ -140,10 +141,10 @@ js::Nursery::init(uint32_t maxNurseryBytes, AutoLockGCBgAlloc& lock) return false; /* maxNurseryBytes parameter is rounded down to a multiple of chunk size. */ - maxNurseryChunks_ = maxNurseryBytes >> ChunkShift; + chunkCountLimit_ = maxNurseryBytes >> ChunkShift; /* If no chunks are specified then the nursery is permanently disabled. */ - if (maxNurseryChunks_ == 0) + if (chunkCountLimit_ == 0) return true; if (!allocateFirstChunk(lock)) @@ -191,7 +192,7 @@ js::Nursery::enable() { MOZ_ASSERT(isEmpty()); MOZ_ASSERT(!runtime()->gc.isVerifyPreBarriersEnabled()); - if (isEnabled() || !maxChunks()) + if (isEnabled() || !chunkCountLimit()) return; { @@ -218,7 +219,10 @@ js::Nursery::disable() return; freeChunksFrom(0); + maxChunkCount_ = 0; + currentEnd_ = 0; + runtime()->gc.storeBuffer().disable(); } @@ -239,7 +243,7 @@ js::Nursery::isEmpty() const void js::Nursery::enterZealMode() { if (isEnabled()) - growAllocableSpace(maxChunks()); + maxChunkCount_ = chunkCountLimit(); } void @@ -305,9 +309,18 @@ js::Nursery::allocate(size_t size) #endif if (currentEnd() < position() + size) { - if (currentChunk_ + 1 == numChunks()) + unsigned chunkno = currentChunk_ + 1; + MOZ_ASSERT(chunkno <= chunkCountLimit()); + MOZ_ASSERT(chunkno <= maxChunkCount()); + MOZ_ASSERT(chunkno <= allocatedChunkCount()); + if (chunkno == maxChunkCount()) return nullptr; - setCurrentChunk(currentChunk_ + 1); + if (MOZ_UNLIKELY(chunkno == allocatedChunkCount())) { + if (!allocateNextChunk(chunkno)) + return nullptr; + MOZ_ASSERT(chunkno < allocatedChunkCount()); + } + setCurrentChunk(chunkno); } void* thing = (void*)position(); @@ -526,6 +539,7 @@ js::Nursery::renderProfileJSON(JSONPrinter& json) const json.property("bytes_used", previousGC.nurseryUsedBytes); json.property("cur_capacity", previousGC.nurseryCapacity); json.property("new_capacity", spaceToEnd()); + json.property("lazy_capacity", allocatedChunkCount() * ChunkSize); json.beginObjectProperty("phase_times"); @@ -688,7 +702,7 @@ js::Nursery::collect(JS::gcreason::Reason reason) // Disable the nursery if the user changed the configuration setting. The // nursery can only be re-enabled by resetting the configurationa and // restarting firefox. - if (maxNurseryChunks_ == 0) + if (chunkCountLimit_ == 0) disable(); endProfile(ProfileKey::Total); @@ -711,7 +725,7 @@ js::Nursery::collect(JS::gcreason::Reason reason) fprintf(stderr, "MinorGC: %20s %5.1f%% %4u ", JS::gcreason::ExplainReason(reason), promotionRate * 100, - numChunks()); + maxChunkCount()); printProfileDurations(profileDurations_); if (reportTenurings_) { @@ -916,18 +930,18 @@ js::Nursery::clear() { #ifdef JS_GC_ZEAL /* Poison the nursery contents so touching a freed object will crash. */ - for (unsigned i = 0; i < numChunks(); i++) + for (unsigned i = 0; i < allocatedChunkCount(); i++) chunk(i).poisonAndInit(runtime(), JS_SWEPT_NURSERY_PATTERN); if (runtime()->hasZealMode(ZealMode::GenerationalGC)) { /* Only reset the alloc point when we are close to the end. */ - if (currentChunk_ + 1 == numChunks()) + if (currentChunk_ + 1 == maxChunkCount()) setCurrentChunk(0); } else #endif { #ifdef JS_CRASH_DIAGNOSTICS - for (unsigned i = 0; i < numChunks(); ++i) + for (unsigned i = 0; i < allocatedChunkCount(); ++i) chunk(i).poisonAndInit(runtime(), JS_SWEPT_NURSERY_PATTERN); #endif setCurrentChunk(0); @@ -940,7 +954,7 @@ js::Nursery::clear() size_t js::Nursery::spaceToEnd() const { - unsigned lastChunk = numChunks() - 1; + unsigned lastChunk = maxChunkCount() - 1; MOZ_ASSERT(lastChunk >= currentStartChunk_); MOZ_ASSERT(currentStartPosition_ - chunk(currentStartChunk_).start() <= NurseryChunkUsableSize); @@ -948,7 +962,7 @@ js::Nursery::spaceToEnd() const size_t bytes = (chunk(currentStartChunk_).end() - currentStartPosition_) + ((lastChunk - currentStartChunk_) * NurseryChunkUsableSize); - MOZ_ASSERT(bytes <= numChunks() * NurseryChunkUsableSize); + MOZ_ASSERT(bytes <= maxChunkCount() * NurseryChunkUsableSize); return bytes; } @@ -956,14 +970,42 @@ js::Nursery::spaceToEnd() const MOZ_ALWAYS_INLINE void js::Nursery::setCurrentChunk(unsigned chunkno) { - MOZ_ASSERT(chunkno < maxChunks()); - MOZ_ASSERT(chunkno < numChunks()); + MOZ_ASSERT(chunkno < chunkCountLimit()); + MOZ_ASSERT(chunkno < allocatedChunkCount()); currentChunk_ = chunkno; position_ = chunk(chunkno).start(); currentEnd_ = chunk(chunkno).end(); chunk(chunkno).poisonAndInit(runtime(), JS_FRESH_NURSERY_PATTERN); } +bool +js::Nursery::allocateNextChunk(const unsigned chunkno) +{ + const unsigned priorCount = allocatedChunkCount(); + const unsigned newCount = priorCount + 1; + + MOZ_ASSERT(chunkno == currentChunk_ + 1); + MOZ_ASSERT(chunkno == allocatedChunkCount()); + MOZ_ASSERT(chunkno < chunkCountLimit()); + MOZ_ASSERT(chunkno < maxChunkCount()); + + if (!chunks_.resize(newCount)) + return false; + + Chunk* newChunk; + { + AutoLockGCBgAlloc lock(runtime()); + newChunk = runtime()->gc.getOrAllocChunk(lock); + } + if (!newChunk) { + chunks_.shrinkTo(priorCount); + return false; + } + + chunks_[chunkno] = NurseryChunk::fromChunk(newChunk); + return true; +} + MOZ_ALWAYS_INLINE void js::Nursery::setStartPosition() { @@ -1000,10 +1042,10 @@ js::Nursery::maybeResizeNursery(JS::gcreason::Reason reason) float(previousGC.tenuredBytes) / float(previousGC.nurseryCapacity); newMaxNurseryChunks = runtime()->gc.tunables.gcMaxNurseryBytes() >> ChunkShift; - if (newMaxNurseryChunks != maxNurseryChunks_) { - maxNurseryChunks_ = newMaxNurseryChunks; + if (newMaxNurseryChunks != chunkCountLimit_) { + chunkCountLimit_ = newMaxNurseryChunks; /* The configured maximum nursery size is changing */ - if (numChunks() > newMaxNurseryChunks) { + if (maxChunkCount() > newMaxNurseryChunks) { /* We need to shrink the nursery */ shrinkAllocableSpace(newMaxNurseryChunks); @@ -1017,46 +1059,16 @@ js::Nursery::maybeResizeNursery(JS::gcreason::Reason reason) // nursery chunks we do not report an error. growAllocableSpace(); } else if (promotionRate < ShrinkThreshold && previousPromotionRate_ < ShrinkThreshold) { - shrinkAllocableSpace(numChunks() - 1); + shrinkAllocableSpace(maxChunkCount() - 1); } previousPromotionRate_ = promotionRate; } -bool +void js::Nursery::growAllocableSpace() { - return growAllocableSpace(Min(numChunks() * 2, maxChunks())); -} - -bool -js::Nursery::growAllocableSpace(unsigned newCount) -{ - unsigned priorCount = numChunks(); - - if (newCount == priorCount) - return false; - - MOZ_ASSERT(newCount > priorCount); - MOZ_ASSERT(newCount <= maxChunks()); - MOZ_ASSERT(priorCount >= 1); - - if (!chunks_.resize(newCount)) - return false; - - AutoLockGCBgAlloc lock(runtime()); - for (unsigned i = priorCount; i < newCount; i++) { - auto newChunk = runtime()->gc.getOrAllocChunk(lock); - if (!newChunk) { - chunks_.shrinkTo(i); - return false; - } - - chunks_[i] = NurseryChunk::fromChunk(newChunk); - chunk(i).poisonAndInit(runtime(), JS_FRESH_NURSERY_PATTERN); - } - - return true; + maxChunkCount_ = Min(maxChunkCount() * 2, chunkCountLimit()); } void @@ -1081,12 +1093,15 @@ js::Nursery::shrinkAllocableSpace(unsigned newCount) // Don't shrink the nursery to zero (use Nursery::disable() instead) and // don't attempt to shrink it to the same size. - if ((newCount == 0) || (newCount == numChunks())) + if ((newCount == 0) || (newCount == maxChunkCount())) return; - MOZ_ASSERT(newCount < numChunks()); + MOZ_ASSERT(newCount < maxChunkCount()); - freeChunksFrom(newCount); + if (newCount < allocatedChunkCount()) + freeChunksFrom(newCount); + + maxChunkCount_ = newCount; } void @@ -1098,11 +1113,12 @@ js::Nursery::minimizeAllocableSpace() bool js::Nursery::allocateFirstChunk(AutoLockGCBgAlloc& lock) { - // This assertion isn't required for correctness, but we do assume this + // These assertions aren't required for correctness, but we do assume this // is only called to initialize or re-enable the nursery. - MOZ_ASSERT(numChunks() == 0); + MOZ_ASSERT(allocatedChunkCount() == 0); + MOZ_ASSERT(maxChunkCount() == 0); - MOZ_ASSERT(maxChunks() > 0); + MOZ_ASSERT(chunkCountLimit() > 0); if (!chunks_.resize(1)) return false; @@ -1114,6 +1130,7 @@ js::Nursery::allocateFirstChunk(AutoLockGCBgAlloc& lock) } chunks_[0] = NurseryChunk::fromChunk(chunk); + maxChunkCount_ = 1; return true; } diff --git a/js/src/gc/Nursery.h b/js/src/gc/Nursery.h index 7a0a2baa61ed..e12f80920cd6 100644 --- a/js/src/gc/Nursery.h +++ b/js/src/gc/Nursery.h @@ -141,14 +141,22 @@ class Nursery MOZ_MUST_USE bool init(uint32_t maxNurseryBytes, AutoLockGCBgAlloc& lock); - unsigned maxChunks() const { return maxNurseryChunks_; } - unsigned numChunks() const { return chunks_.length(); } + unsigned chunkCountLimit() const { return chunkCountLimit_; } - bool exists() const { return maxChunks() != 0; } + // Number of allocated (ready to use) chunks. + unsigned allocatedChunkCount() const { return chunks_.length(); } + + // Total number of chunks and the capacity of the nursery. Chunks will be + // lazilly allocated and added to the chunks array up to this limit, after + // that the nursery must be collected, this limit may be raised during + // collection. + unsigned maxChunkCount() const { return maxChunkCount_; } + + bool exists() const { return chunkCountLimit() != 0; } void enable(); void disable(); - bool isEnabled() const { return numChunks() != 0; } + bool isEnabled() const { return maxChunkCount() != 0; } /* Return true if no allocations have been made since the last collection. */ bool isEmpty() const; @@ -233,7 +241,7 @@ class Nursery MOZ_MUST_USE bool queueDictionaryModeObjectToSweep(NativeObject* obj); size_t sizeOfHeapCommitted() const { - return numChunks() * gc::ChunkSize; + return allocatedChunkCount() * gc::ChunkSize; } size_t sizeOfMallocedBuffers(mozilla::MallocSizeOf mallocSizeOf) const { if (!mallocedBuffers.initialized()) @@ -252,7 +260,7 @@ class Nursery MOZ_ALWAYS_INLINE size_t freeSpace() const { MOZ_ASSERT(currentEnd_ - position_ <= NurseryChunkUsableSize); return (currentEnd_ - position_) + - (numChunks() - currentChunk_ - 1) * NurseryChunkUsableSize; + (maxChunkCount() - currentChunk_ - 1) * NurseryChunkUsableSize; } #ifdef JS_GC_ZEAL @@ -310,8 +318,18 @@ class Nursery /* The index of the chunk that is currently being allocated from. */ unsigned currentChunk_; - /* Maximum number of chunks to allocate for the nursery. */ - unsigned maxNurseryChunks_; + /* + * The nursery may grow the chunks_ vector up to this size without a + * collection. This allows the nursery to grow lazilly. This limit may + * change during maybeResizeNursery() each collection. + */ + unsigned maxChunkCount_; + + /* + * This limit is fixed by configuration. It represents the maximum size + * the nursery is permitted to tune itself to in maybeResizeNursery(); + */ + unsigned chunkCountLimit_; /* Promotion rate for the previous minor collection. */ float previousPromotionRate_; @@ -431,6 +449,8 @@ class Nursery */ MOZ_MUST_USE bool allocateFirstChunk(AutoLockGCBgAlloc& lock); + MOZ_MUST_USE bool allocateNextChunk(unsigned chunkno); + MOZ_ALWAYS_INLINE uintptr_t currentEnd() const; uintptr_t position() const { return position_; } @@ -481,13 +501,12 @@ class Nursery /* Change the allocable space provided by the nursery. */ void maybeResizeNursery(JS::gcreason::Reason reason); - bool growAllocableSpace(); - bool growAllocableSpace(unsigned newSize); + void growAllocableSpace(); void shrinkAllocableSpace(unsigned newCount); void minimizeAllocableSpace(); // Free the chunks starting at firstFreeChunk until the end of the chunks - // vector. Shrinks the vector but does not update maxChunksAlloc(). + // vector. Shrinks the vector but does not update maxChunkCount(). void freeChunksFrom(unsigned firstFreeChunk); /* Profile recording and printing. */ From 33001bfd325e3a681f111a555577ed4718e3b6b5 Mon Sep 17 00:00:00 2001 From: Paul Bone Date: Mon, 30 Oct 2017 17:46:23 +1100 Subject: [PATCH 11/13] Bug 1298018 - Part 5: Remove allocateFirstChunk(). r=jonco --- js/src/gc/Nursery.cpp | 49 +++++++++++++------------------------------ js/src/gc/Nursery.h | 9 ++++---- 2 files changed, 19 insertions(+), 39 deletions(-) diff --git a/js/src/gc/Nursery.cpp b/js/src/gc/Nursery.cpp index 86e83236eca9..6132c8accb68 100644 --- a/js/src/gc/Nursery.cpp +++ b/js/src/gc/Nursery.cpp @@ -147,8 +147,12 @@ js::Nursery::init(uint32_t maxNurseryBytes, AutoLockGCBgAlloc& lock) if (chunkCountLimit_ == 0) return true; - if (!allocateFirstChunk(lock)) + maxChunkCount_ = 1; + if (!allocateNextChunk(0, lock)) { + maxChunkCount_ = 0; return false; + } + /* After this point the Nursery has been enabled */ setCurrentChunk(0); setStartPosition(); @@ -197,8 +201,11 @@ js::Nursery::enable() { AutoLockGCBgAlloc lock(runtime()); - if (!allocateFirstChunk(lock)) + maxChunkCount_ = 1; + if (!allocateNextChunk(0, lock)) { + maxChunkCount_ = 0; return; + } } setCurrentChunk(0); @@ -316,7 +323,8 @@ js::Nursery::allocate(size_t size) if (chunkno == maxChunkCount()) return nullptr; if (MOZ_UNLIKELY(chunkno == allocatedChunkCount())) { - if (!allocateNextChunk(chunkno)) + AutoLockGCBgAlloc lock(runtime()); + if (!allocateNextChunk(chunkno, lock)) return nullptr; MOZ_ASSERT(chunkno < allocatedChunkCount()); } @@ -979,12 +987,13 @@ js::Nursery::setCurrentChunk(unsigned chunkno) } bool -js::Nursery::allocateNextChunk(const unsigned chunkno) +js::Nursery::allocateNextChunk(const unsigned chunkno, + AutoLockGCBgAlloc& lock) { const unsigned priorCount = allocatedChunkCount(); const unsigned newCount = priorCount + 1; - MOZ_ASSERT(chunkno == currentChunk_ + 1); + MOZ_ASSERT((chunkno == currentChunk_ + 1) || (chunkno == 0 && allocatedChunkCount() == 0)); MOZ_ASSERT(chunkno == allocatedChunkCount()); MOZ_ASSERT(chunkno < chunkCountLimit()); MOZ_ASSERT(chunkno < maxChunkCount()); @@ -993,10 +1002,7 @@ js::Nursery::allocateNextChunk(const unsigned chunkno) return false; Chunk* newChunk; - { - AutoLockGCBgAlloc lock(runtime()); - newChunk = runtime()->gc.getOrAllocChunk(lock); - } + newChunk = runtime()->gc.getOrAllocChunk(lock); if (!newChunk) { chunks_.shrinkTo(priorCount); return false; @@ -1110,31 +1116,6 @@ js::Nursery::minimizeAllocableSpace() shrinkAllocableSpace(1); } -bool -js::Nursery::allocateFirstChunk(AutoLockGCBgAlloc& lock) -{ - // These assertions aren't required for correctness, but we do assume this - // is only called to initialize or re-enable the nursery. - MOZ_ASSERT(allocatedChunkCount() == 0); - MOZ_ASSERT(maxChunkCount() == 0); - - MOZ_ASSERT(chunkCountLimit() > 0); - - if (!chunks_.resize(1)) - return false; - - auto chunk = runtime()->gc.getOrAllocChunk(lock); - if (!chunk) { - chunks_.shrinkTo(0); - return false; - } - - chunks_[0] = NurseryChunk::fromChunk(chunk); - maxChunkCount_ = 1; - - return true; -} - bool js::Nursery::queueDictionaryModeObjectToSweep(NativeObject* obj) { diff --git a/js/src/gc/Nursery.h b/js/src/gc/Nursery.h index e12f80920cd6..d52d60e8628f 100644 --- a/js/src/gc/Nursery.h +++ b/js/src/gc/Nursery.h @@ -444,12 +444,11 @@ class Nursery void setStartPosition(); /* - * Ensure that the first chunk has been allocated. Callers will probably - * want to call setCurrentChunk(0) next. + * Allocate the next chunk, or the first chunk for initialization. + * Callers will probably want to call setCurrentChunk(0) next. */ - MOZ_MUST_USE bool allocateFirstChunk(AutoLockGCBgAlloc& lock); - - MOZ_MUST_USE bool allocateNextChunk(unsigned chunkno); + MOZ_MUST_USE bool allocateNextChunk(unsigned chunkno, + AutoLockGCBgAlloc& lock); MOZ_ALWAYS_INLINE uintptr_t currentEnd() const; From ff87dafa787dd13fc5fe32f380a687fc3d8b321f Mon Sep 17 00:00:00 2001 From: JerryShih Date: Mon, 6 Nov 2017 16:07:22 +0800 Subject: [PATCH 12/13] Bug 1409176 - make SyncObjectD3D11Client become fallible. r=dvander Currently, the device-reset flow doesn't notify the decoder for device change immediately. The decoder might use an invalid sync-object for synchronization. Then, we will hit some assertions. This patch try to make the synchronization flow become fallible, then we could pass the error to the media framework for error handling. MozReview-Commit-ID: BFY32MmOdt0 --- gfx/layers/SyncObject.h | 12 ++++++---- gfx/layers/d3d11/TextureD3D11.cpp | 38 ++++++++++++++++++++----------- gfx/layers/d3d11/TextureD3D11.h | 4 ++-- 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/gfx/layers/SyncObject.h b/gfx/layers/SyncObject.h index e7c454570a72..874b30f75445 100644 --- a/gfx/layers/SyncObject.h +++ b/gfx/layers/SyncObject.h @@ -28,14 +28,15 @@ public: static already_AddRefed CreateSyncObjectHost( #ifdef XP_WIN - ID3D11Device* aDevice = nullptr + ID3D11Device* aDevice = nullptr #endif - ); + ); virtual bool Init() = 0; virtual SyncHandle GetSyncHandle() = 0; + // Return false for failed synchronization. virtual bool Synchronize() = 0; protected: @@ -50,9 +51,9 @@ public: static already_AddRefed CreateSyncObjectClient(SyncHandle aHandle #ifdef XP_WIN - , ID3D11Device* aDevice = nullptr + , ID3D11Device* aDevice = nullptr #endif - ); + ); enum class SyncType { D3D11, @@ -60,7 +61,8 @@ public: virtual SyncType GetSyncType() = 0; - virtual void Synchronize() = 0; + // Return false for failed synchronization. + virtual bool Synchronize(bool aFallible = false) = 0; virtual bool IsSyncObjectValid() = 0; diff --git a/gfx/layers/d3d11/TextureD3D11.cpp b/gfx/layers/d3d11/TextureD3D11.cpp index ad95eeeb1104..407f589723e5 100644 --- a/gfx/layers/d3d11/TextureD3D11.cpp +++ b/gfx/layers/d3d11/TextureD3D11.cpp @@ -1697,7 +1697,7 @@ SyncObjectD3D11Client::SyncObjectD3D11Client(SyncHandle aSyncHandle, ID3D11Devic } bool -SyncObjectD3D11Client::Init() +SyncObjectD3D11Client::Init(bool aFallible) { if (mKeyedMutex) { return true; @@ -1709,7 +1709,7 @@ SyncObjectD3D11Client::Init() (void**)(ID3D11Texture2D**)getter_AddRefs(mSyncTexture)); if (FAILED(hr) || !mSyncTexture) { gfxCriticalNote << "Failed to OpenSharedResource for SyncObjectD3D11: " << hexa(hr); - if (ShouldDevCrashOnSyncInitFailure()) { + if (!aFallible && ShouldDevCrashOnSyncInitFailure()) { gfxDevCrash(LogReason::D3D11FinalizeFrame) << "Without device reset: " << hexa(hr); } return false; @@ -1719,8 +1719,13 @@ SyncObjectD3D11Client::Init() if (FAILED(hr) || !mKeyedMutex) { // Leave both the critical error and MOZ_CRASH for now; the critical error lets // us "save" the hr value. We will probably eventually replace this with gfxDevCrash. - gfxCriticalError() << "Failed to get KeyedMutex (2): " << hexa(hr); - MOZ_CRASH("GFX: Cannot get D3D11 KeyedMutex"); + if (!aFallible) { + gfxCriticalError() << "Failed to get KeyedMutex (2): " << hexa(hr); + MOZ_CRASH("GFX: Cannot get D3D11 KeyedMutex"); + } else { + gfxCriticalNote << "Failed to get KeyedMutex (3): " << hexa(hr); + } + return false; } return true; @@ -1747,8 +1752,8 @@ SyncObjectD3D11Client::IsSyncObjectValid() // into our sync object and only use a lock for this sync object. // This way, we don't have to sync every texture we send to the compositor. // We only have to do this once per transaction. -void -SyncObjectD3D11Client::Synchronize() +bool +SyncObjectD3D11Client::Synchronize(bool aFallible) { // Since this can be called from either the Paint or Main thread. // We don't want this to race since we initialize the sync texture here @@ -1756,10 +1761,10 @@ SyncObjectD3D11Client::Synchronize() MutexAutoLock syncLock(mSyncLock); if (!mSyncedTextures.size()) { - return; + return true; } - if (!Init()) { - return; + if (!Init(aFallible)) { + return false; } HRESULT hr; @@ -1768,9 +1773,14 @@ SyncObjectD3D11Client::Synchronize() if (hr == WAIT_TIMEOUT) { if (DeviceManagerDx::Get()->HasDeviceReset()) { gfxWarning() << "AcquireSync timed out because of device reset."; - return; + return false; } - gfxDevCrash(LogReason::D3D11SyncLock) << "Timeout on the D3D11 sync lock"; + if (aFallible) { + gfxWarning() << "Timeout on the D3D11 sync lock."; + } else { + gfxDevCrash(LogReason::D3D11SyncLock) << "Timeout on the D3D11 sync lock."; + } + return false; } D3D11_BOX box; @@ -1782,13 +1792,13 @@ SyncObjectD3D11Client::Synchronize() if (dev == DeviceManagerDx::Get()->GetContentDevice()) { if (DeviceManagerDx::Get()->HasDeviceReset()) { - return; + return false; } } if (dev != mDevice) { gfxWarning() << "Attempt to sync texture from invalid device."; - return; + return false; } RefPtr ctx; @@ -1799,6 +1809,8 @@ SyncObjectD3D11Client::Synchronize() } mSyncedTextures.clear(); + + return true; } uint32_t diff --git a/gfx/layers/d3d11/TextureD3D11.h b/gfx/layers/d3d11/TextureD3D11.h index 035e52fdc0d8..2858268ec833 100644 --- a/gfx/layers/d3d11/TextureD3D11.h +++ b/gfx/layers/d3d11/TextureD3D11.h @@ -478,7 +478,7 @@ class SyncObjectD3D11Client : public SyncObjectClient public: explicit SyncObjectD3D11Client(SyncHandle aSyncHandle, ID3D11Device* aDevice); - virtual void Synchronize() override; + virtual bool Synchronize(bool aFallible) override; virtual bool IsSyncObjectValid() override; @@ -487,7 +487,7 @@ public: void RegisterTexture(ID3D11Texture2D* aTexture); private: - bool Init(); + bool Init(bool aFallible); SyncHandle mSyncHandle; RefPtr mDevice; From bc746b55a9f48ce86d62a370eca93ef26ad36777 Mon Sep 17 00:00:00 2001 From: JerryShih Date: Mon, 6 Nov 2017 16:07:22 +0800 Subject: [PATCH 13/13] Bug 1409176 - handle decoder error in WMFMediaDataDecoder::ProcessDecode(). r=jya,mattwoodrow The DXVA decoder might hit some error because of the hardware device status. This patch try to pass the error code to the decoder promise object to deal with the error. MozReview-Commit-ID: IAj8gzKGF3j --- dom/media/platforms/wmf/DXVA2Manager.cpp | 6 ++++-- dom/media/platforms/wmf/WMFMediaDataDecoder.cpp | 11 +++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/dom/media/platforms/wmf/DXVA2Manager.cpp b/dom/media/platforms/wmf/DXVA2Manager.cpp index 112aa40cfb38..575d4d8b1717 100644 --- a/dom/media/platforms/wmf/DXVA2Manager.cpp +++ b/dom/media/platforms/wmf/DXVA2Manager.cpp @@ -929,7 +929,7 @@ D3D11DXVA2Manager::CopyToImage(IMFSample* aVideoSample, NS_ENSURE_TRUE(client, E_FAIL); RefPtr mutex; - HRESULT hr; + HRESULT hr = S_OK; RefPtr texture = image->GetTexture(); texture->QueryInterface((IDXGIKeyedMutex**)getter_AddRefs(mutex)); @@ -987,7 +987,9 @@ D3D11DXVA2Manager::CopyToImage(IMFSample* aVideoSample, // It appears some race-condition may allow us to arrive here even when mSyncObject // is null. It's better to avoid that crash. client->SyncWithObject(mSyncObject); - mSyncObject->Synchronize(); + if (!mSyncObject->Synchronize(true)) { + return DXGI_ERROR_DEVICE_RESET; + } } image.forget(aOutImage); diff --git a/dom/media/platforms/wmf/WMFMediaDataDecoder.cpp b/dom/media/platforms/wmf/WMFMediaDataDecoder.cpp index 65ec9598ae14..405bd467bc09 100644 --- a/dom/media/platforms/wmf/WMFMediaDataDecoder.cpp +++ b/dom/media/platforms/wmf/WMFMediaDataDecoder.cpp @@ -115,6 +115,10 @@ WMFMediaDataDecoder::ProcessError(HRESULT aError, const char* aReason) SendTelemetry(aError); mRecordedError = true; } + + //TODO: For the error DXGI_ERROR_DEVICE_RESET, we could return + // NS_ERROR_DOM_MEDIA_NEED_NEW_DECODER to get the latest device. Maybe retry + // up to 3 times. return DecodePromise::CreateAndReject( MediaResult(NS_ERROR_DOM_MEDIA_DECODE_ERR, RESULT_DETAIL("%s:%x", aReason, aError)), @@ -127,7 +131,10 @@ WMFMediaDataDecoder::ProcessDecode(MediaRawData* aSample) DecodedData results; HRESULT hr = mMFTManager->Input(aSample); if (hr == MF_E_NOTACCEPTING) { - ProcessOutput(results); + hr = ProcessOutput(results); + if (FAILED(hr)) { + return ProcessError(hr, "MFTManager::Output(1)"); + } hr = mMFTManager->Input(aSample); } @@ -143,7 +150,7 @@ WMFMediaDataDecoder::ProcessDecode(MediaRawData* aSample) if (SUCCEEDED(hr) || hr == MF_E_TRANSFORM_NEED_MORE_INPUT) { return DecodePromise::CreateAndResolve(Move(results), __func__); } - return ProcessError(hr, "MFTManager::Output"); + return ProcessError(hr, "MFTManager::Output(2)"); } HRESULT