1508565 - 2.17% Resident Memory (android-em-4-3-armv7-api16) regression on push 9a2cf737f3e3d623ad99857070f4646e4be45388 (Sat Nov 17 2018)

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect, P2)

Description

[Ionuț Goldan [:igoldan]]

We have detected an awsy regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c21bf3a5e752044715f0026f8407eed642c794c6&tochange=9a2cf737f3e3d623ad99857070f4646e4be45388

As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

  2%  Resident Memory android-em-4-3-armv7-api16 opt      142,049,979.46 -> 145,131,527.42


You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=17645

On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the jobs in a pushlog format.

To learn more about the regressing test(s), please see: https://wiki.mozilla.org/AWSY/Tests

Comment 1

[Thomas Wisniewski [:twisniewski]]

Andrew, is this the kind of RAM-use regression we would expect from a webextension-ization of the webcompat reporter on Fennec?

Comment 2

[Andrew Swan [:aswan]]

(In reply to Thomas Wisniewski [:twisniewski] from comment #1)
> Andrew, is this the kind of RAM-use regression we would expect from a
> webextension-ization of the webcompat reporter on Fennec?

Maybe?  Assuming the units in comment 0 are bytes this means we're now using an extra 3MB.  If somebody has the time and inclination to do it, gathering and comparing memory dumps with and without webcompart-reporter to see exactly where the memory is going would be the next step.  Also adding Kris in case he has any thoughts to share.

Comment 3

[Eric Rahm [:erahm]]

(In reply to Andrew Swan [:aswan] from comment #2)
> (In reply to Thomas Wisniewski [:twisniewski] from comment #1)
> > Andrew, is this the kind of RAM-use regression we would expect from a
> > webextension-ization of the webcompat reporter on Fennec?
> 
> Maybe?  Assuming the units in comment 0 are bytes this means we're now using
> an extra 3MB.  If somebody has the time and inclination to do it, gathering
> and comparing memory dumps with and without webcompart-reporter to see
> exactly where the memory is going would be the next step.  Also adding Kris
> in case he has any thoughts to share.

Thomas, can you look into gathering the memory reports? The existing test is pretty simple, you should be able to modify the checkpoint code [1] to dump a full memory report (MemoryObserver has an example [2]) and grab those out of the devices tmp dir.

[1] https://searchfox.org/mozilla-central/rev/55895c49f55073d82d977cb74ec1d3a71ae4b25f/mobile/android/tests/browser/chrome/test_awsy_lite.html#42-46
[2] https://searchfox.org/mozilla-central/rev/55895c49f55073d82d977cb74ec1d3a71ae4b25f/mobile/android/chrome/content/MemoryObserver.js#65-67

Comment 4

[Thomas Wisniewski [:twisniewski]]

How would I grab them out of the tmp dir? Do I need vpn/ssh access to the test box in question (I don't think I have either with my access rights)?

Comment 5

[Eric Rahm [:erahm]]

(In reply to Thomas Wisniewski [:twisniewski] from comment #4)
> How would I grab them out of the tmp dir? Do I need vpn/ssh access to the
> test box in question (I don't think I have either with my access rights)?

I'd suggest testing locally, otherwise you'll need to figure out how we package artifacts for this type of test and write the report to that dir with `dumpMemoryReportsToNamedFile` [1]. gbrown might be able to help with that.

[1] https://searchfox.org/mozilla-central/rev/55895c49f55073d82d977cb74ec1d3a71ae4b25f/xpcom/base/nsIMemoryInfoDumper.idl#86-89

Comment 6

[Geoff Brown [:gbrown]]

Running locally should be simple: If you have a local android build, just:

  $ ./mach mochitest mobile/android/tests/browser/chrome/test_awsy_lite.html

That will offer to run an emulator if you don't have a device attached or emulator running already. You can also use 'mach android-emulator --version 4.3' first to start the emulator with the same android image used in CI.

Use 'adb shell' to get shell access to the emulator; 'adb pull' to transfer files from the emulator.


To do the same thing on try would be tricky, but this reminds me of bug 1483478; I'll have a look at that, but may not have a solution in time for this bug.

Comment 7

[Kris Maglione [:kmag]]

It's probably worth noting that you really need to apply mccr8's patch for the heap dumps to be useful...

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Moving this into the Firefox for Android product so we can properly track this regression.

Comment 9

[Ionuț Goldan [:igoldan]]

Any updates here?

Comment 10

[Thomas Wisniewski [:twisniewski]]

No updates yet, I'm afraid. I'll investigate this ASAP.

Comment 11

[Mike Taylor [:miketaylr]]

Do we need to track this for release? The Report Site Issue feature is Beta and Nightly only.

See also https://bugzilla.mozilla.org/show_bug.cgi?id=1452783.

Comment 12

[Mike Taylor [:miketaylr]]

(better link https://searchfox.org/mozilla-central/source/mobile/android/extensions/moz.build#13)

Comment 13

[Mike Taylor [:miketaylr]]

ni? for Comment #11

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

Looks like we are just tracking for beta 65. I will leave that up to Ryan.

Comment 15

[Mike Taylor [:miketaylr]]

So this seems like another instance of memory overhead for booting up any webextension, not really tied to the webcompat reporter. 

I was going to set this as disabled for 65, since this won't ship to release... but that feels wrong since it's enabled while 65 is in Beta. Ryan, what's the best way to fiddle flags here?

Comment 16

[Ryan VanderMeulen [:RyanVM]]

I'll flip it to disabled after 65 is merged to release in a couple weeks.

Comment 17

[Liz Henry (:lizzard) (relman/hg->git project)]

Thomas are you working on this for 66 nightly?

Comment 18

[Thomas Wisniewski [:twisniewski]]

No, and based on comment 15 I'm not sure what needs to be done. Did we still need any investigation or other work done here?

Comment 19

[Eric Rahm [:erahm]]

Lets just WONTFIX this given it's not going to release.