Benjamin Smedberg Reporter
	Description • 7 years ago

I had some really bad pauses today. I'm not sure which site(s) might be causing this, and I had trouble reading the profile so I'm going to file this and hopefully get some help understanding the profile.

https://perfht.ml/2oPN69Z <- profile
about:memory attached

	Benjamin Smedberg Reporter
	Comment 1 • 7 years ago

Questions about profile reading:
There are some really long pauses identified in the upper section with hover titles like "8.NNN second event processing delay on tab thread". But it's not clear to me how to figure out which of the four content processes that is and then how to get the profile of that process but not the rest.

	(no longer active)
	Comment 2 • 7 years ago

Talked with Benjamin about this on IRC, these are CC pauses...

	Olli Pettay [:smaug][bugs@pettay.fi] Assignee
	Comment 3 • 7 years ago

Based on the memory report, there are 60 ghost windows.
Gecko profiler profiles aren't really useful with leaks.
CC/GC logs from about:memory might reveal the issue.

	Andrew McCreight [:mccr8]
	Comment 4 • 7 years ago

Yeah, you have hundreds of ghost windows across your various processes. Odds are there is some addon that is causing this.

	Andrew McCreight [:mccr8]
	Comment 5 • 7 years ago

Are you on Firefox 55? What addons do you have installed?

	Benjamin Smedberg Reporter
	Comment 6 • 7 years ago

This was very recent nightly 55 win64. At the time I was running the following:

* Mass Password Reset
* User Agent Switcher
* Moo Later
* Mind the Time

I have since:

installed "Gecko Profiler"
uninstalled "Moo Later" and "Mind the Time"

I will report as I browse today whether I see more ghost windows or similar symptoms.

	Andrew McCreight [:mccr8]
	Comment 7 • 7 years ago

Honestly, with that many ghost windows, you were probably leaking every single one you opened. So just browsing for a few minutes and checking about:memory should tell you something. I think I've never seen so many ghost windows in a single memory report, and I've seen a lot of memory reports. ;)

	Benjamin Smedberg Reporter
	Comment 8 • 7 years ago

Browsing today with the shorter addon list, still seeing ghost windows. memory dump attached

	Andrew McCreight [:mccr8]
	Comment 9 • 7 years ago

Which "User Agent Switcher" addon do you use? I see 4 with that name and various hypenations.
The most popular one is https://addons.mozilla.org/en-US/firefox/addon/user-agent-switcher/?src=search so I'll try that.

	Kan-Ru Chen [:kanru] (UTC+9)
	Comment 10 • 7 years ago

I'm not sure if it's same issue but I've got 400+ ghost windows

	Andrew McCreight [:mccr8]
	Comment 11 • 7 years ago

(In reply to Kan-Ru Chen [:kanru] (UTC+8) from comment #10)
> I'm not sure if it's same issue but I've got 400+ ghost windows

Do you have any of the same addons as listed in comment 6? If not, please file a new bug and needinfo me.

	Andrew McCreight [:mccr8]
	Comment 12 • 7 years ago

(In reply to Andrew McCreight [:mccr8] from comment #9)
> Which "User Agent Switcher" addon do you use? I see 4 with that name and
> various hypenations.

Also, do you have any rules or whatever set up for it? I tried browsing a little with the user agent switcher and mass password reset addons installed but I didn't get any leaks. Though I should try it with the profiler, too.

	Benjamin Smedberg Reporter
	Comment 13 • 7 years ago

I have the one with ID {e968fc70-8f95-4ab9-9e79-304de2a71ee1}

I'm not actually sure how to tell whether I have any site-specific overrides set up.

Also I don't think I'm using this addon currently, so let me try disabling it as well.

	Olli Pettay [:smaug][bugs@pettay.fi] Assignee
	Comment 14 • 7 years ago

I'm seeing ghosts locally. I don't know how to reproduce the issue, but atm
the regression range looks to be 
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c697e756f738ce37abc56f31bfbc48f55625d617&tochange=bb38d935d699e0529f9e0bb35578d381026415c4

I got some ghosts with 2017-04-18-10-02-13-mozilla-central (bb38d935d699e0529f9e0bb35578d381026415c4) but so far haven't got using 
2017-04-17-10-02-36-mozilla-central (c697e756f738ce37abc56f31bfbc48f55625d617)

	Olli Pettay [:smaug][bugs@pettay.fi] Assignee
	Comment 15 • 7 years ago

telemetry seems to hint about something similar. There is a spike in 04-18.

https://telemetry.mozilla.org/new-pipeline/evo.html#!aggregates=median!mean!5th-percentile!25th-percentile!75th-percentile!95th-percentile&cumulative=0&end_date=2017-04-10&keys=!__none__!__none__&max_channel_version=nightly%252F55&measure=GHOST_WINDOWS&min_channel_version=nightly%252F55&os=Windows_NT&processType=*&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2017-03-08&trim=1&use_submission_date=0

	Olli Pettay [:smaug][bugs@pettay.fi] Assignee
	Comment 16 • 7 years ago

Also https://telemetry.mozilla.org/new-pipeline/evo.html#!aggregates=median!mean!5th-percentile!25th-percentile!75th-percentile!95th-percentile&cumulative=0&end_date=2017-04-10&keys=!__none__!__none__&max_channel_version=nightly%252F55&measure=CYCLE_COLLECTOR_MAX_PAUSE&min_channel_version=nightly%252F55&os=Windows_NT&processType=*&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2017-03-08&trim=1&use_submission_date=0 seems to hint there is a regression between 04-17 and 04-18.

	Olli Pettay [:smaug][bugs@pettay.fi] Assignee
	Comment 17 • 7 years ago

a374c3546993 does seem to leak.

	Dave Townsend [:mossop]
	Comment 18 • 7 years ago

I think I'm seeing this too, is there any information I can provide to help?

	Olli Pettay [:smaug][bugs@pettay.fi] Assignee
	Comment 19 • 7 years ago

STR would be really nice here. I can occasionally reproduce this locally using my main FF profile.
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c697e756f738ce37abc56f31bfbc48f55625d617&tochange=bb38d935d699e0529f9e0bb35578d381026415c4 is probably the right regression range.

Oddly enough https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cac374fb0de1&tochange=a374c3546993 seems to be more precise range, but that doesn't quite make sense.
Could be that I just haven't seen the leak.

Bug 1358979 doesn't seem to help here.

	Asa Dotzler [:asa]
	Comment 20 • 7 years ago

I'm seeing this with no addons so updating the summary.

	(no longer active)
	Comment 21 • 7 years ago

(In reply to Olli Pettay [:smaug] from comment #19)
> Oddly enough
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=cac374fb0de1&tochange=a374c3546993 seems to be more
> precise range, but that doesn't quite make sense.

Could bug 1357069 be a candidate?

	Olli Pettay [:smaug][bugs@pettay.fi] Assignee
	Comment 22 • 7 years ago

I haven't opened any pdfs, but perhaps pdf.js accesses all the pages somehow.

	Andrew Overholt [:overholt]
	Comment 23 • 7 years ago

I have been experiencing this a lot and also have no addons other than the profiler enabled.

Here's my about:memory (you can see some closed-earlier-today Google Slides and Sheets URLs) and my profile is: https://perfht.ml/2ptKnWU

	Olli Pettay [:smaug][bugs@pettay.fi] Assignee
	Comment 24 • 7 years ago

I'm going through changesets now one by one. Just need to browse random pages after a build is ready, since I still don't have STR, so this takes time.
If anyone has some STR (which might just work locally with a certain FF profile), help would be appreciated.

	Benjamin Smedberg Reporter
	Comment 25 • 7 years ago

With my main profile, STR is still:
* launch profile with restored session consisting of 7 pinned tabs
* open a new tab and load something
* close the new tab

	Andrew McCreight [:mccr8]
	Comment 26 • 7 years ago

(In reply to Olli Pettay [:smaug] from comment #24)
> I'm going through changesets now one by one. Just need to browse random
> pages after a build is ready, since I still don't have STR, so this takes
> time.

Have you run find_roots.py on the CC edges file yet? What does that turn up?

[Tracking Requested - why for this release]: bad leaks many people are seeing

	Olli Pettay [:smaug][bugs@pettay.fi] Assignee
	Comment 27 • 7 years ago

I think I found it. r=smaug patch :p
/me tests.

	(no longer active)
	Comment 28 • 7 years ago

(In reply to Olli Pettay [:smaug] from comment #27)
> I think I found it. r=smaug patch :p
> /me tests.

Ugh, nsDocShell::GetPresContext() addrefs!

	Andrew McCreight [:mccr8]
	Comment 30 • 7 years ago

[Tracking Requested - why for this release]:

	Andrew McCreight [:mccr8]
	Comment 31 • 7 years ago

Thanks for tracking this down, Olli!

	Ryan VanderMeulen [:RyanVM]
	Comment 32 • 7 years ago

Aha, maybe this bug *does* explain why I've been seeing 54.0b1 doing the same thing also then! I really think this should block 54.0b2, Gerry. My experience over the weekend is that this was giving me an extremely janky, nearly unusable browser.

	Pulsebot
	Comment 33 • 7 years ago

Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/363b1e7eccc3
nsPresContext is refcounted, so use it so, r=kats

	Olli Pettay [:smaug][bugs@pettay.fi] Assignee
	Comment 34 • 7 years ago

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

(Since I don't know whether one is supposed to ask approval for aurora or beta, asking both.)

Approval Request Comment
[Feature/Bug causing the regression]: bug 1356695
[User impact if declined]: Major leaks
[Is this code covered by automated tests?]: NA
[Has the fix been verified in Nightly?]: NA
[Needs manual test from QE? If yes, steps to reproduce]: Load pages and then
about:memory Measure and look for ghosts
[Is the change risky?]:Not risky
[Why is the change risky/not risky?]: just using refcounted object correctly
[String changes made/needed]:NA

	Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)
	Comment 35 • 7 years ago

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

Let's get this on m-b now, so that we may still be able to build beta 2 tonight.

	Ryan VanderMeulen [:RyanVM]
	Comment 36 • 7 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/ca3c6131b6ea

	Carsten Book [:Tomcat]
	Comment 37 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/363b1e7eccc3

	Andrei Vaida [:avaida]
	Comment 38 • 7 years ago

Flagging this for manual testing, instructions in Comment 34.

	Andrew McCreight [:mccr8]
	Comment 39 • 7 years ago

(In reply to Andrei Vaida [:avaida], Desktop Release QA – please ni? me from comment #38)
> Flagging this for manual testing, instructions in Comment 34.

The testing probably needs to be done on a computer with a touch screen, and it requires using the touch screen.

	Olli Pettay [:smaug][bugs@pettay.fi] Assignee
	Comment 40 • 7 years ago

Does need touchscreen, but doesn't need using it. It just needs to be enabled and pages need to do something to try to use touch events or something.