diff --git a/browser/devtools/performance/docs/markers.md b/browser/devtools/performance/docs/markers.md index c81cb43f3cb..1f13b4da0e5 100644 --- a/browser/devtools/performance/docs/markers.md +++ b/browser/devtools/performance/docs/markers.md @@ -107,8 +107,7 @@ a setTimeout. ## GarbageCollection -Emitted after a full GC cycle has completed (which is after any number of -incremental slices). +Emitted after a full GC has occurred (which will emit past incremental events). * DOMString causeName - The reason for a GC event to occur. A full list of GC reasons can be found [on MDN](https://developer.mozilla.org/en-US/docs/Tools/Debugger-API/Debugger.Memory#Debugger.Memory_Handler_Functions). @@ -116,17 +115,6 @@ incremental slices). GC (smaller, quick GC events), and we have to walk the entire heap and GC everything marked, then the reason listed here is why. -## nsCycleCollector::Collect - -An `nsCycleCollector::Collect` marker is emitted for each incremental cycle -collection slice and each non-incremental cycle collection. - -# nsCycleCollector::ForgetSkippable - -`nsCycleCollector::ForgetSkippable` is presented as "Cycle Collection", but in -reality it is preparation/pre-optimization for cycle collection, and not the -actual tracing of edges and collecting of cycles. - ## ConsoleTime A marker generated via `console.time()` and `console.timeEnd()`. diff --git a/browser/devtools/performance/modules/logic/marker-utils.js b/browser/devtools/performance/modules/logic/marker-utils.js index 43608eaac9d..345359202df 100644 --- a/browser/devtools/performance/modules/logic/marker-utils.js +++ b/browser/devtools/performance/modules/logic/marker-utils.js @@ -435,13 +435,6 @@ const Formatters = { return { "Restyle Hint": marker.restyleHint.replace(/eRestyle_/g, "") }; } }, - - CycleCollectionFields: function (marker) { - let Type = PREFS["show-platform-data"] - ? marker.name - : marker.name.replace(/nsCycleCollector::/g, ""); - return { Type }; - }, }; exports.getMarkerLabel = getMarkerLabel; diff --git a/browser/devtools/performance/modules/markers.js b/browser/devtools/performance/modules/markers.js index 984eb4aa79b..6f9a4a33e2b 100644 --- a/browser/devtools/performance/modules/markers.js +++ b/browser/devtools/performance/modules/markers.js @@ -113,20 +113,6 @@ const TIMELINE_BLUEPRINT = { { property: "nonincrementalReason", label: "Non-incremental Reason:" } ], }, - "nsCycleCollector::Collect": { - group: 1, - colorName: "graphs-red", - collapseFunc: either(collapse.parent, collapse.child), - label: "Cycle Collection", - fields: Formatters.CycleCollectionFields, - }, - "nsCycleCollector::ForgetSkippable": { - group: 1, - colorName: "graphs-red", - collapseFunc: either(collapse.parent, collapse.child), - label: "Cycle Collection", - fields: Formatters.CycleCollectionFields, - }, /* Group 2 - User Controlled */ "ConsoleTime": { diff --git a/browser/devtools/performance/test/browser.ini b/browser/devtools/performance/test/browser.ini index e5ee8c5dd2e..7c21b500542 100644 --- a/browser/devtools/performance/test/browser.ini +++ b/browser/devtools/performance/test/browser.ini @@ -2,7 +2,6 @@ tags = devtools subsuite = devtools support-files = - doc_force_cc.html doc_force_gc.html doc_innerHTML.html doc_markers.html @@ -14,7 +13,6 @@ support-files = [browser_aaa-run-first-leaktest.js] [browser_marker-utils.js] -[browser_markers-cycle-collection.js] [browser_markers-gc.js] [browser_markers-parse-html.js] [browser_markers-styles.js] diff --git a/browser/devtools/performance/test/browser_markers-cycle-collection.js b/browser/devtools/performance/test/browser_markers-cycle-collection.js deleted file mode 100644 index cd266c7cfc5..00000000000 --- a/browser/devtools/performance/test/browser_markers-cycle-collection.js +++ /dev/null @@ -1,47 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -/** - * Test that we get "nsCycleCollector::Collect" and - * "nsCycleCollector::ForgetSkippable" markers when we force cycle collection. - */ - -const TEST_URL = EXAMPLE_URL + "doc_force_cc.html" - -function waitForMarkerType(front, type) { - info("Waiting for marker of type = " + type); - const { promise, resolve } = Promise.defer(); - - const handler = (_, name, markers) => { - if (name !== "markers") { - return; - } - - info("Got markers: " + JSON.stringify(markers, null, 2)); - - if (markers.some(m => m.name === type)) { - ok(true, "Found marker of type = " + type); - front.off("timeline-data", handler); - resolve(); - } - }; - front.on("timeline-data", handler); - - return promise; -} - -function* spawnTest () { - let { target, front } = yield initBackend(TEST_URL); - - yield front.startRecording({ withMarkers: true, withTicks: true }); - - yield Promise.all([ - waitForMarkerType(front, "nsCycleCollector::Collect"), - waitForMarkerType(front, "nsCycleCollector::ForgetSkippable") - ]); - ok(true, "Got expected cycle collection events"); - - yield front.stopRecording(); - yield removeTab(target.tab); - finish(); -} diff --git a/browser/devtools/performance/test/doc_force_cc.html b/browser/devtools/performance/test/doc_force_cc.html deleted file mode 100644 index d5868bd6b7c..00000000000 --- a/browser/devtools/performance/test/doc_force_cc.html +++ /dev/null @@ -1,29 +0,0 @@ - - - - - - - Performance tool + cycle collection test page - - - - - - - diff --git a/docshell/base/AutoTimelineMarker.cpp b/docshell/base/AutoTimelineMarker.cpp deleted file mode 100644 index 5a2027d9741..00000000000 --- a/docshell/base/AutoTimelineMarker.cpp +++ /dev/null @@ -1,105 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=2 et sw=2 tw=80: */ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#include "mozilla/AutoTimelineMarker.h" - -#include "MainThreadUtils.h" -#include "nsDocShell.h" -#include "mozilla/Move.h" - -namespace mozilla { - -bool -AutoTimelineMarker::DocShellIsRecording(nsDocShell& aDocShell) -{ - bool isRecording = false; - if (nsDocShell::gProfileTimelineRecordingsCount > 0) { - aDocShell.GetRecordProfileTimelineMarkers(&isRecording); - } - return isRecording; -} - -AutoTimelineMarker::AutoTimelineMarker(nsIDocShell* aDocShell, const char* aName - MOZ_GUARD_OBJECT_NOTIFIER_PARAM_IN_IMPL) - : mDocShell(nullptr) - , mName(aName) -{ - MOZ_GUARD_OBJECT_NOTIFIER_INIT; - MOZ_ASSERT(NS_IsMainThread()); - - nsDocShell* docShell = static_cast(aDocShell); - if (docShell && DocShellIsRecording(*docShell)) { - mDocShell = docShell; - mDocShell->AddProfileTimelineMarker(mName, TRACING_INTERVAL_START); - } -} - -AutoTimelineMarker::~AutoTimelineMarker() -{ - if (mDocShell) { - mDocShell->AddProfileTimelineMarker(mName, TRACING_INTERVAL_END); - } -} - -void -AutoGlobalTimelineMarker::PopulateDocShells() -{ - const LinkedList& docShells = - nsDocShell::GetObservedDocShells(); - MOZ_ASSERT(!docShells.isEmpty()); - - for (const nsDocShell::ObservedDocShell* ds = docShells.getFirst(); - ds; - ds = ds->getNext()) { - mOk = mDocShells.append(**ds); - if (!mOk) { - return; - } - } -} - -AutoGlobalTimelineMarker::AutoGlobalTimelineMarker(const char* aName - MOZ_GUARD_OBJECT_NOTIFIER_PARAM_IN_IMPL) - : mOk(true) - , mDocShells() - , mName(aName) -{ - MOZ_GUARD_OBJECT_NOTIFIER_INIT; - MOZ_ASSERT(NS_IsMainThread()); - - if (nsDocShell::gProfileTimelineRecordingsCount == 0) { - return; - } - - PopulateDocShells(); - if (!mOk) { - // If we don't successfully populate our vector with *all* docshells being - // observed, don't add markers to *any* of them. - return; - } - - for (Vector>::Range range = mDocShells.all(); - !range.empty(); - range.popFront()) { - range.front()->AddProfileTimelineMarker(mName, TRACING_INTERVAL_START); - } -} - -AutoGlobalTimelineMarker::~AutoGlobalTimelineMarker() -{ - if (!mOk) { - return; - } - - for (Vector>::Range range = mDocShells.all(); - !range.empty(); - range.popFront()) { - range.front()->AddProfileTimelineMarker(mName, TRACING_INTERVAL_END); - } -} - - -} // namespace mozilla diff --git a/docshell/base/AutoTimelineMarker.h b/docshell/base/AutoTimelineMarker.h index 2f9db74158a..c0c4da18369 100644 --- a/docshell/base/AutoTimelineMarker.h +++ b/docshell/base/AutoTimelineMarker.h @@ -8,13 +8,10 @@ #define AutoTimelineMarker_h__ #include "mozilla/GuardObjects.h" -#include "mozilla/Vector.h" - +#include "mozilla/Move.h" +#include "nsDocShell.h" #include "nsRefPtr.h" -class nsIDocShell; -class nsDocShell; - namespace mozilla { // # AutoTimelineMarker @@ -30,6 +27,7 @@ namespace mozilla { // nsresult rv = ParseTheCSSFile(mFile); // ... // } + class MOZ_STACK_CLASS AutoTimelineMarker { MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER; @@ -37,57 +35,42 @@ class MOZ_STACK_CLASS AutoTimelineMarker nsRefPtr mDocShell; const char* mName; - bool DocShellIsRecording(nsDocShell& aDocShell); + bool + DocShellIsRecording(nsDocShell& aDocShell) + { + bool isRecording = false; + if (nsDocShell::gProfileTimelineRecordingsCount > 0) { + aDocShell.GetRecordProfileTimelineMarkers(&isRecording); + } + return isRecording; + } public: explicit AutoTimelineMarker(nsIDocShell* aDocShell, const char* aName - MOZ_GUARD_OBJECT_NOTIFIER_PARAM); - ~AutoTimelineMarker(); + MOZ_GUARD_OBJECT_NOTIFIER_PARAM) + : mDocShell(nullptr) + , mName(aName) + { + MOZ_GUARD_OBJECT_NOTIFIER_INIT; + + nsDocShell* docShell = static_cast(aDocShell); + if (docShell && DocShellIsRecording(*docShell)) { + mDocShell = docShell; + mDocShell->AddProfileTimelineMarker(mName, TRACING_INTERVAL_START); + } + } + + ~AutoTimelineMarker() + { + if (mDocShell) { + mDocShell->AddProfileTimelineMarker(mName, TRACING_INTERVAL_END); + } + } AutoTimelineMarker(const AutoTimelineMarker& aOther) = delete; void operator=(const AutoTimelineMarker& aOther) = delete; }; -// # AutoGlobalTimelineMarker -// -// Similar to `AutoTimelineMarker`, but adds its traced marker to all docshells, -// not a single particular one. This is useful for operations that aren't -// associated with any one particular doc shell, or when it isn't clear which -// doc shell triggered the operation. -// -// Example usage: -// -// { -// AutoGlobalTimelineMarker marker("Cycle Collection"); -// nsCycleCollector* cc = GetCycleCollector(); -// cc->Collect(); -// ... -// } -class MOZ_STACK_CLASS AutoGlobalTimelineMarker -{ - MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER; - - // True as long as no operation has failed, eg due to OOM. - bool mOk; - - // The set of docshells that are being observed and will get markers. - mozilla::Vector> mDocShells; - - // The name of the marker we are adding. - const char* mName; - - void PopulateDocShells(); - -public: - explicit AutoGlobalTimelineMarker(const char* aName - MOZ_GUARD_OBJECT_NOTIFIER_PARAM); - - ~AutoGlobalTimelineMarker(); - - AutoGlobalTimelineMarker(const AutoGlobalTimelineMarker& aOther) = delete; - void operator=(const AutoGlobalTimelineMarker& aOther) = delete; -}; - } // namespace mozilla #endif /* AutoTimelineMarker_h__ */ diff --git a/docshell/base/moz.build b/docshell/base/moz.build index 602089b8e6d..12b9b0e786f 100644 --- a/docshell/base/moz.build +++ b/docshell/base/moz.build @@ -48,7 +48,6 @@ EXPORTS.mozilla += [ ] UNIFIED_SOURCES += [ - 'AutoTimelineMarker.cpp', 'LoadContext.cpp', 'nsAboutRedirector.cpp', 'nsDefaultURIFixup.cpp', diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index f5bcd8cdd69..2a839b0b906 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -944,7 +944,7 @@ nsDocShell::nsDocShell() nsDocShell::~nsDocShell() { - MOZ_ASSERT(!IsObserved()); + MOZ_ASSERT(!mProfileTimelineRecording); Destroy(); @@ -2927,8 +2927,6 @@ nsDocShell::HistoryTransactionRemoved(int32_t aIndex) unsigned long nsDocShell::gProfileTimelineRecordingsCount = 0; -mozilla::LinkedList* nsDocShell::gObservedDocShells = nullptr; - NS_IMETHODIMP nsDocShell::SetRecordProfileTimelineMarkers(bool aValue) { @@ -2937,16 +2935,11 @@ nsDocShell::SetRecordProfileTimelineMarkers(bool aValue) if (aValue) { ++gProfileTimelineRecordingsCount; UseEntryScriptProfiling(); - - MOZ_ASSERT(!mObserved); - mObserved.reset(new ObservedDocShell(this)); - GetOrCreateObservedDocShells().insertFront(mObserved.get()); + mProfileTimelineRecording = true; } else { --gProfileTimelineRecordingsCount; UnuseEntryScriptProfiling(); - - mObserved.reset(nullptr); - + mProfileTimelineRecording = false; ClearProfileTimelineMarkers(); } } @@ -2957,7 +2950,7 @@ nsDocShell::SetRecordProfileTimelineMarkers(bool aValue) NS_IMETHODIMP nsDocShell::GetRecordProfileTimelineMarkers(bool* aValue) { - *aValue = IsObserved(); + *aValue = mProfileTimelineRecording; return NS_OK; } @@ -3097,7 +3090,7 @@ void nsDocShell::AddProfileTimelineMarker(const char* aName, TracingMetadata aMetaData) { - if (IsObserved()) { + if (mProfileTimelineRecording) { TimelineMarker* marker = new TimelineMarker(this, aName, aMetaData); mProfileTimelineMarkers.AppendElement(marker); } @@ -3106,7 +3099,7 @@ nsDocShell::AddProfileTimelineMarker(const char* aName, void nsDocShell::AddProfileTimelineMarker(UniquePtr&& aMarker) { - if (IsObserved()) { + if (mProfileTimelineRecording) { mProfileTimelineMarkers.AppendElement(Move(aMarker)); } } diff --git a/docshell/base/nsDocShell.h b/docshell/base/nsDocShell.h index 3f376c39905..7c6d1cad70b 100644 --- a/docshell/base/nsDocShell.h +++ b/docshell/base/nsDocShell.h @@ -23,7 +23,6 @@ #include "mozilla/TimeStamp.h" #include "GeckoProfiler.h" #include "mozilla/dom/ProfileTimelineMarkerBinding.h" -#include "mozilla/LinkedList.h" #include "jsapi.h" // Helper Classes @@ -267,43 +266,6 @@ public: // timeline markers static unsigned long gProfileTimelineRecordingsCount; - class ObservedDocShell : public mozilla::LinkedListElement - { - public: - explicit ObservedDocShell(nsDocShell* aDocShell) - : mDocShell(aDocShell) - { } - - nsDocShell* operator*() const { return mDocShell.get(); } - - private: - nsRefPtr mDocShell; - }; - -private: - static mozilla::LinkedList* gObservedDocShells; - - static mozilla::LinkedList& GetOrCreateObservedDocShells() - { - if (!gObservedDocShells) { - gObservedDocShells = new mozilla::LinkedList(); - } - return *gObservedDocShells; - } - - // Never null if timeline markers are being observed. - mozilla::UniquePtr mObserved; - - // Return true if timeline markers are being observed for this docshell. False - // otherwise. - bool IsObserved() const { return !!mObserved; } - -public: - static const mozilla::LinkedList& GetObservedDocShells() - { - return GetOrCreateObservedDocShells(); - } - // Tell the favicon service that aNewURI has the same favicon as aOldURI. static void CopyFavicon(nsIURI* aOldURI, nsIURI* aNewURI, @@ -1011,6 +973,9 @@ private: // has been called without a matching NotifyRunToCompletionStop. uint32_t mJSRunToCompletionDepth; + // True if recording profiles. + bool mProfileTimelineRecording; + nsTArray> mProfileTimelineMarkers; // Get rid of all the timeline markers accumulated so far diff --git a/xpcom/base/nsCycleCollector.cpp b/xpcom/base/nsCycleCollector.cpp index 1919f0aaf76..a50ed8be468 100644 --- a/xpcom/base/nsCycleCollector.cpp +++ b/xpcom/base/nsCycleCollector.cpp @@ -182,7 +182,6 @@ #include #include -#include "mozilla/AutoTimelineMarker.h" #include "mozilla/Likely.h" #include "mozilla/PoisonIOInterposer.h" #include "mozilla/Telemetry.h" @@ -2813,11 +2812,6 @@ nsCycleCollector::ForgetSkippable(bool aRemoveChildlessNodes, { CheckThreadSafety(); - mozilla::Maybe marker; - if (NS_IsMainThread()) { - marker.emplace("nsCycleCollector::ForgetSkippable"); - } - // If we remove things from the purple buffer during graph building, we may // lose track of an object that was mutated during graph building. MOZ_ASSERT(mIncrementalPhase == IdlePhase); @@ -3578,11 +3572,6 @@ nsCycleCollector::Collect(ccType aCCType, MOZ_ASSERT(!IsIncrementalGCInProgress()); - mozilla::Maybe marker; - if (NS_IsMainThread()) { - marker.emplace("nsCycleCollector::Collect"); - } - bool startedIdle = (mIncrementalPhase == IdlePhase); bool collectedAny = false;