Similar to bug 1137844, but for cycle collection rather than garbage collection.

So it looks like the place to gather this data and then post a runnable to fire this hook for observing debuggers would be in `nsJSContext::EndCycleCollectionCallback`. I'm under the impression that CC is also incremental, but it isn't clear to me if this callback is called at the end of each CC slice or at the end of a CC cycle. I think the former? If so, then it will be more difficult to implement the same per-cycle API we have for onGarbageCollection. However, some Debugger API users have requested that onGarbageCollection switch to firing per-slice, so maybe it makes sense to fire this hook per-slice and then update the onGarbageCollection hook to be per-slice as well in the future. We don't seem to have reasons for CC, like we do for GC. CycleCollectorStats uses Timestamp, which is great as that's what the SPS profiler and markers use. Soon the onGarbageCollection hook will use it too: bug 1159506.

The two CC callbacks in nsJSContext are called at the very start and the very end of the cycle collection.

What information does the debugger need? We have the observer notifications already, it is fired in EndCycleCollectionCallback. Can we reuse that? Or does debugger want to get called whenever we start/end a CC slice? If so, I think also forgetSkippable slices should be considered. (forgetSkippable slices run before the actual incremental CC. forgetSkippable is about optimizing out certainly alive objects from the CC graph. It needs several slices since part of the optimization is black-bit-propagation, propagating the information about certainly-aliveness through C++->JS->C++ boundaries. In some cases it can take quite some time, but it reduces CC time dramatically. Per telemetry 95th percentile CYCLE_COLLECTOR_MAX_PAUSE is ~50ms, and FORGET_SKIPPABLE_MAX ~16ms, medians 6ms and 3ms)

(In reply to Olli Pettay [:smaug] from comment #3) Yeah, it made sense for onGarbageCollection to hang off of Debugger because that's just how we do APIs exposing SpiderMonkey's internals to JS. Then I never rethought that decision with this bug. The initial goal is to trace cycle collection in the devtools' profiler's waterfall view. I realized after hacking around a bit yesterday that instead of jumping through hoops with an onCycleCollection hook, we can do tracing the usual way here: timeline markers via AutoTimelineMarker. The onGarbageCollection hook is being used for more than just tracing GCs (it provides a unit of time for draining the allocations log), but the only thing we want from CCs is tracing. +1 for reflection, -1 for clarity of this bug; sorry! The only catch is that cycle collection is a global operation as opposed to being associated with a specific window/document (like the timeline profiler markers expect), so I plan to just add the CC marker to every docshell that is being traced. So, my plan is now: 1) Maintain a `mozilla::LinkedList` of docshells actively being traced by devtools or the gecko profiler addon. 2) Add `AutoGlobalTimelineMarker` which goes through the above list and adds a timeline profiler marker to each of them. 3) Use `AutoGlobalTimelineMarker` in `nsCycleCollector::Collect` (and not differentiate between cycle collection phases, at least for now)

Note, nsCycleCollector::Collect doesn't catch forgetSkippable calls.

(In reply to Olli Pettay [:smaug] from comment #5) > Note, nsCycleCollector::Collect doesn't catch forgetSkippable calls. Would it make sense to add AutoGlobalTimelineMarker in nsCycleCollector::ForgetSkippable, too? Or is there a better place where we can catch both actions with one AutoGlobalTimelineMarker? From some DXR digging, it appears the ForgetSkippable is called by nsJSContext::BeginCycleCollectionCallback, which is called by XPCJSRuntime::BeginCycleCollectionCallback, which is a virtual override that's called by nsCycleCollector::BeginCycleCollection, which is called by nsCycleCollector::Collect. So at least this path for ForgetSkippable *would* fall under Collect. There are a couple others, but I haven't dug into them all yet.

ForgetSkippable is mostly called on its own timer, separate from the CC. It is like a pre-CC clean up phase. The one in BeginCCCallback does the clean up in case we did not run that before.

This patch series exposes ForgetSkippable as "Minor Cycle Collection" for webdevs. :mccr8 says that isn't quite correct and that it is more like "Cycle Collection Helper", but I just don't think that rolls off the tongue very well :P Any alternative ideas?

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=366fc40d364b

Comment on attachment 8613823 [details] [diff] [review] Part 4: Expose cycle collection markers in the devtools frontend Review of attachment 8613823 [details] [diff] [review]: ----------------------------------------------------------------- Few things: * I don't think we should bother labeling these events differently as minor vs normal. We can add a "Type: Collect" and "Type: ForgetSkippable" fields if inspecting the marker, but wonder how useful it is, from a bird's eye view in the waterfall, to have two distinctly different CC markers. * This should be labeled as "CC Event", to be consistent with the "GC Event". If we want to point out one is more important (like the GC Event non-incremental), then let's do that consistently with the GC events. For the label, can define a formatter: function (marker) { let generic = L10N.getStr("timeline.label.cyclecollection"); if (marker.name === "nsCycleCollector::Collect") { return `${generic} (major)`; } return generic; } But would rather the metadata contain extra information rather than more text on the waterfall, using a function in the fields prop as: function (marker) { let type = marker.name === "nsCycleCollect::Collect" ? "Collection" : "Helper"; return { Type: type }; // Name the property "Type" whatever should be displayed. } These markers are way too platform-specific to localize, if anyone did localize in the first place. Let me know if you have any questions -- pretty much want to make it as clean as possible, considering most people don't even know what CC is. We can hide more data behind gecko platform pref if we want, too. Would be nice for a follow up to have maybe a (?) in the marker details on some of these markers explaining what CC is maybe.

Comment on attachment 8613821 [details] [diff] [review] Part 5: Add a test for cycle collection markers Review of attachment 8613821 [details] [diff] [review]: ----------------------------------------------------------------- The GC test runs GC in the chrome thread on e10s, which might not be the best way to do it -- the result is sometimes it takes a bit to trigger on e10s (so disabled on e10s for now) -- does this suffer the same issue on e10s? If it takes a few seconds longer than you'd think on e10s, put it under a conditional to not run there. Otherwise, good to go

Can handle any frontend stuff in bug 1128083 to not hold up this landing too, btw

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #17) > Comment on attachment 8613821 [details] [diff] [review] > Part 5: Add a test for cycle collection markers > > Review of attachment 8613821 [details] [diff] [review]: > ----------------------------------------------------------------- > > The GC test runs GC in the chrome thread on e10s, which might not be the > best way to do it -- the result is sometimes it takes a bit to trigger on > e10s (so disabled on e10s for now) -- does this suffer the same issue on > e10s? If it takes a few seconds longer than you'd think on e10s, put it > under a conditional to not run there. Otherwise, good to go This test doesn't have that problem: note how it is the content tab that calls SpecialPowers.Cu.forceCC() not the (chrome) test itself. Filed bug 1170609 to make the GC markers test do something similar.

Comment on attachment 8613817 [details] [diff] [review] Part 1: Maintain a list of docshells whose timeline markers are being observed >+#include "mozilla/Move.h" drop this > nsDocShell::~nsDocShell() > { >- MOZ_ASSERT(!mProfileTimelineRecording); >+ MOZ_ASSERT(!IsObserved()); Hmm, IsObserved() is quite generic name, but I don't have now better suggestions. > NS_IMETHODIMP > nsDocShell::SetRecordProfileTimelineMarkers(bool aValue) > { > bool currentValue = nsIDocShell::GetRecordProfileTimelineMarkers(); > if (currentValue != aValue) { > if (aValue) { > ++gProfileTimelineRecordingsCount; > UseEntryScriptProfiling(); >- mProfileTimelineRecording = true; >+ >+ MOZ_ASSERT(!mObserved); >+ mObserved = new ObservedDocShell( >+ mozilla::Move(nsRefPtr<nsDocShell>(this))); >+ GetObservedDocShellsMutable().insertFront(mObserved); > } else { > --gProfileTimelineRecordingsCount; > UnuseEntryScriptProfiling(); >- mProfileTimelineRecording = false; >+ >+ delete mObserved; >+ mObserved = nullptr; manual deletion looks bad. Use nsAutoPtr or UniquePtr? >@@ -261,16 +262,52 @@ public: > // See nsIDocShell::recordProfileTimelineMarkers > void AddProfileTimelineMarker(const char* aName, TracingMetadata aMetaData); > void AddProfileTimelineMarker(mozilla::UniquePtr<TimelineMarker>&& aMarker); > > // Global counter for how many docShells are currently recording profile > // timeline markers > static unsigned long gProfileTimelineRecordingsCount; > >+ class ObservedDocShell : public mozilla::LinkedListElement<ObservedDocShell> >+ { >+ nsRefPtr<nsDocShell> mDocShell; >+ >+ public: >+ explicit ObservedDocShell(nsRefPtr<nsDocShell>&& aDocShell) >+ : mDocShell(Move(aDocShell)) >+ { } I'd prefer member variables at the end of class declaration. And why all this odd Move() handling. Just nsDocShell* aDocShell as a parameter, and then since mDocShell is nsRefPtr it will keep the object alive. >+ static mozilla::LinkedList<ObservedDocShell>& GetObservedDocShellsMutable() Call this GetOrCreateObservedDocShells(). (GetOrCreate* is rather commonly used prefix in Gecko for this type of method) I'd like to see a new patch without Move() magic which just makes code harder to read (and write).

Note: should add an entry to browser/devtools/performance/docs/markers.md

Comment on attachment 8613818 [details] [diff] [review] Part 2: Add mozilla::AutoGlobalTimelineMarker >+AutoGlobalTimelineMarker::PopulateDocShells() { { to its own line >+ auto& docShells = nsDocShell::GetObservedDocShells(); auto just makes code harder to read. Please use the actual type. >+ MOZ_ASSERT(!docShells.isEmpty()); >+ >+ for (auto* ds = docShells.getFirst(); ds; ds = ds->getNext()) { >+ mOk = mDocShells.append(mozilla::Move(nsRefPtr<nsDocShell>(**ds))); auto* ds makes this hard to read. I don't know what kind of thing it is. >+ for (auto range = mDocShells.all(); !range.empty(); range.popFront()) { >+ range.front()->AddProfileTimelineMarker(mName, TRACING_INTERVAL_START); >+ } sorry, hard to sell use of auto to me. It just makes code harder to read, pretty much in all the cases. (auto is great for *writing* code, but bad for readability, IMHO) >+AutoGlobalTimelineMarker::~AutoGlobalTimelineMarker() >+{ >+ if (!mOk) { >+ return; >+ } >+ >+ for (auto range = mDocShells.all(); !range.empty(); range.popFront()) { and here too. Since mDocShells sounds like nsDocShell[] or such, I'd expect range to be nsDocShell* >+ range.front()->AddProfileTimelineMarker(mName, TRACING_INTERVAL_END); ... but apparently it is something else since nsDocShell doesn't have front()

Comment on attachment 8613819 [details] [diff] [review] Part 3: Trace cycle collection with AutoGlobalTimelineMarker I wonder if we should have different kinds of markers for incremental CC slices and full CC. Though, doesn't matter usually.

Comment on attachment 8614302 [details] [diff] [review] Part 4: Expose cycle collection markers in the devtools frontend Review of attachment 8614302 [details] [diff] [review]: ----------------------------------------------------------------- r+ with the below changes; your call on best name for the property to display the type of field, and whether or not we should hide the field completely when geckomode off, just strip off "nsCycleCollection", or just dump the full name. For the deep view like this, it's not going to make much sense anyway for non-gecko hackers unless we have helpers anyway ::: browser/devtools/performance/docs/markers.md @@ +115,5 @@ > * DOMString nonincremenetalReason - If the GC could not do an incremental > GC (smaller, quick GC events), and we have to walk the entire heap and > GC everything marked, then the reason listed here is why. > > +## Cycle Collection (nsCycleCollector::Collect and nsCycleCollector::ForgetSkippable) Would prefer these to be separate entries with their full marker names -- docs are mostly for devtools frontenders who want to know what data is on the markers themselves, before massaging, without having to dump out a bunch of markers that may have different properties (like logging a bunch of GC markers to see all the different options that are available, so one could format them) ::: browser/devtools/performance/modules/logic/marker-utils.js @@ +419,5 @@ > } > }, > + > + CycleCollectionLabel: function (marker) { > + return PREFS["show-platform-data"] ? marker.name : "Cycle Collection"; What we discussed on IRC: Let's leave the label always "Cycle Collection". For the detailed info: CycleCollectionFields: function (marker) { let Type = PREFS["show-platform-data"] ? marker.name : marker.name.replace(/nsCycleCollector/i, ""); return { Type }; } Or whatever field name you think is more appropriate than "Type", and can just dump the name itself here I think if geckomode on. In not geckomode, can just strip out the nsCycleCollector stuff, but still esoteric regardless, so don't feel too strongly about hiding info here (as it is details view) ::: browser/devtools/performance/modules/markers.js @@ +122,5 @@ > }, > + "nsCycleCollector::Collect": { > + group: 1, > + colorName: "graphs-red", > + collapseFunc: collapseConsecutiveIdentical, gonna need a rebase on these collapseFuncs ::: browser/locales/en-US/chrome/browser/devtools/timeline.properties @@ +40,5 @@ > timeline.label.reflow2=Layout > timeline.label.paint=Paint > timeline.label.javascript2=Function Call > +timeline.label.cycleCollection=Cycle Collection > +timeline.label.forgetSkippable=Minor Cycle Collection Can remove these now

Comment on attachment 8614300 [details] [diff] [review] Part 1: Maintain a list of docshells whose timeline markers are being observed > #include "mozilla/EventStateManager.h" >+#include "mozilla/Move.h" Why you need this?

Comment on attachment 8614301 [details] [diff] [review] Part 2: Add mozilla::AutoGlobalTimelineMarker + bool mOk; Could you please add some documentation what mOk means. >+ mozilla::Vector<nsRefPtr<nsDocShell>> mDocShells; ugh, Vector. I guess I can live with it this time. This is the only use of Vector in dom/ or docshell/ though. I even thought Vector is deprecated the same way as mfbt/RefPtr

This series has all review comments addressed, but after rebasing on m-c, I keep hitting crashes immediately upon starting the profiler. Will look into it tomorrow. [NPAPI 39698] ###!!! ABORT: Aborting on channel error.: file /Users/fitzgen/src/mozilla-central/ipc/glue/MessageChannel.cpp, line 1662 #01: mozilla::ipc::MessageChannel::OnChannelErrorFromLink()[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x82dea4] #02: mozilla::ipc::ProcessLink::OnChannelError()[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x82f7f1] #03: non-virtual thunk to mozilla::ipc::ProcessLink::OnChannelError()[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x82f81c] #04: IPC::Channel::ChannelImpl::OnFileCanReadWithoutBlocking(int)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7f3fec] #05: base::MessagePumpLibevent::OnLibeventNotification(int, short, void*)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7a648e] #06: event_persist_closure[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7957ca] #07: event_process_active_single_queue[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x795020] #08: event_process_active[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7798ac] #09: event_base_loop[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7789a7] #10: base::MessagePumpLibevent::Run(base::MessagePump::Delegate*)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7a68cc] #11: MessageLoop::RunInternal()[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7a43b5] #12: MessageLoop::RunHandler()[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7a42c5] #13: MessageLoop::Run()[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7a426d] #14: base::Thread::ThreadMain()[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7c9686] #15: ThreadFunc(void*)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7ca85e] #16: _pthread_body[/usr/lib/system/libsystem_pthread.dylib +0x3268] #17: _pthread_body[/usr/lib/system/libsystem_pthread.dylib +0x31e5] [NPAPI 39698] ###!!! ABORT: Aborting on channel error.: file /Users/fitzgen/src/mozilla-central/ipc/glue/MessageChannel.cpp, line 1662 Hit MOZ_CRASH() at /Users/fitzgen/src/mozilla-central/memory/mozalloc/mozalloc_abort.cpp:33

Comment on attachment 8614436 [details] [diff] [review] Part 4: Expose cycle collection markers in the devtools frontend Review of attachment 8614436 [details] [diff] [review]: ----------------------------------------------------------------- With bug 1169439 landed, you might want to change the collapse function for these to be similar to the GC markers (child/parent folding, rather than consecutive-similar)

Hrm, I don't get the ipc error under gdb, instead I get this crash within the profiler: Assertion failure: strlen(mChunkList[i].get()) == mChunkLengths[i], at /Users/fitzgen/src/mozilla-central/tools/profiler/ProfileJSONWriter.cpp:44 #01: ChunkedJSONWriteFunc::CopyData() const[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4729a80] #02: TableTicker::ToJSON(float)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x472cb9c] #03: TableTicker::ToJSObject(JSContext*, float)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x472c9fc] #04: mozilla_sampler_get_profile_data(JSContext*, float)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4732978] #05: profiler_get_profile_jsobject(JSContext*, float)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x472faaf] #06: nsProfiler::GetProfileData(float, JSContext*, JS::MutableHandle<JS::Value>)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x472f9ff] #07: NS_InvokeByIndex[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1c1771] #08: CallMethodHelper::Invoke()[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x10c59c8] #09: CallMethodHelper::Call()[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x10b6241] #10: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x10953be] #11: XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1097531] #12: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5e6b368] #13: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5df0752] #14: Interpret(JSContext*, js::RunState&)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5e0be4b] #15: js::RunScript(JSContext*, js::RunState&)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5dfe4e2] #16: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5df0890] #17: js::CallOrConstructBoundFunction(JSContext*, unsigned int, JS::Value*)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x6586870] #18: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5e6b368] #19: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5df0752] #20: Interpret(JSContext*, js::RunState&)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5e0be4b] #21: js::RunScript(JSContext*, js::RunState&)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5dfe4e2] #22: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5df0890] #23: js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5dd613e] #24: js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x616cc65]

Gah, that was a stupid bug that happened while renaming methods in part 1. Here's the fix: static const mozilla::LinkedList<ObservedDocShell>& GetObservedDocShells() { - return GetObservedDocShells(); + return GetOrCreateObservedDocShells(); }

Adding dep on bug 1169439 so I can rebase once that is merged to m-c.

remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/63199013dac2 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/f49e64db4a66 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/fff492fc48ce remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/9becec7879dc remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/1db2b848699a

Backed out for browser_timelineMarkers-02.js failures. https://treeherder.mozilla.org/logviewer.html#?job_id=10548632&repo=mozilla-inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/492669c0f158

A test expected only TimeStamp markers, but we were cycle collecting in the middle of the test which screwed it up. New try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7eff101c0e83

Gah, none of these tests expect a cycle collection, and so they are all intermittently failing. Will need to filter the cycle collection markers out for all of them.

Alright this latest rendition filters out CC markers before passing them off to the tests that don't expect them to exist. https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3c5c53a70fa

https://hg.mozilla.org/integration/mozilla-inbound/rev/8b14bf7268ea https://hg.mozilla.org/integration/mozilla-inbound/rev/e7ab2d47bdd9 https://hg.mozilla.org/integration/mozilla-inbound/rev/44e54f229bf8 https://hg.mozilla.org/integration/mozilla-inbound/rev/0f7be7f07b63 https://hg.mozilla.org/integration/mozilla-inbound/rev/56ede9a5462d

https://hg.mozilla.org/mozilla-central/rev/8b14bf7268ea https://hg.mozilla.org/mozilla-central/rev/e7ab2d47bdd9 https://hg.mozilla.org/mozilla-central/rev/44e54f229bf8 https://hg.mozilla.org/mozilla-central/rev/0f7be7f07b63 https://hg.mozilla.org/mozilla-central/rev/56ede9a5462d