801719 - Unmark certainly alive event listener and message managers

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

https://tbpl.mozilla.org/?tree=Try&rev=1c936a7c52c9

Comment 1

[Andrew McCreight [:mccr8]]

Comment on attachment 671476 [details] [diff] [review]
patch

Review of attachment 671476 [details] [diff] [review]:
-----------------------------------------------------------------

Uck, all this null point juggling is gross, but I guess it will do for now.

::: content/base/src/nsCCUncollectableMarker.cpp
@@ +91,5 @@
>  
>  static void
>  MarkMessageManagers()
>  {
> +  nsCOMPtr<nsIMessageBroadcaster> stronGlobalMM =

|stronGlobalMM| should be |strongGlobalMM|

@@ +147,5 @@
>    }
> +  if (nsFrameMessageManager::sParentProcessManager) {
> +    nsFrameMessageManager::sParentProcessManager->MarkForCC();
> +    uint32_t childCount = 0;
> +    nsFrameMessageManager::sParentProcessManager->GetChildCount(&childCount);

Why isn't all of this child marking stuff done normally as part of MarkForCC()?  Is it usually not worth doing?

::: content/events/src/nsEventListenerManager.h
@@ +225,5 @@
>    bool MayHaveMouseEnterLeaveEventListener() { return mMayHaveMouseEnterLeaveEventListener; }
>  
>    size_t SizeOfIncludingThis(nsMallocSizeOfFun aMallocSizeOf) const;
>  
> +  void UnmarkGrayJSListenersAndRemovePurple();

Should this become a more generic name like MarkForCC() or CleanupForCC()?

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Andrew McCreight [:mccr8] from comment #1)
> Uck, all this null point juggling is gross
indeed

> |stronGlobalMM| should be |strongGlobalMM|
One day, which is not any time this decade, I'll learn to type English.
(and I'll learn to speak English a decaded after that.)

> Why isn't all of this child marking stuff done normally as part of
> MarkForCC()?  Is it usually not worth doing?
Gets tricky when dealing with process and frame mm. I prefer keeping the hacks here.
I guess that is the only reason.

> Should this become a more generic name like MarkForCC() or CleanupForCC()?
ok, I'll change to MarkForCC

Comment 3

[Andrew McCreight [:mccr8]]

(In reply to Olli Pettay [:smaug] from comment #2)
> > |stronGlobalMM| should be |strongGlobalMM|
> One day, which is not any time this decade, I'll learn to type English.
> (and I'll learn to speak English a decaded after that.)

Your English is very good!  It is just your variable names that are occasionally slightly odd.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/mozilla-central/rev/5a6a36d109d7

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Based on telemetry this affected surprisingly positively to median
CYCLE_COLLECTOR_VISITED_GCED.

(Separate issue is CYCLE_COLLECTOR_VISITED_REF_COUNTED regression end of September.)