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

RESOLVED WONTFIX

Status

()

defect
P2
normal
RESOLVED WONTFIX
7 months ago
5 months ago

People

(Reporter: igoldan, Unassigned)

Tracking

(Blocks 1 bug, {perf, regression})

unspecified
Unspecified
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox63 unaffected, firefox64 unaffected, firefox65- wontfix, firefox66- wontfix)

Details

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
Product: Testing → Web Compatibility Tools
Flags: needinfo?(twisniewski)
Andrew, is this the kind of RAM-use regression we would expect from a webextension-ization of the webcompat reporter on Fennec?
Flags: needinfo?(twisniewski) → needinfo?(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.
Flags: needinfo?(aswan)

Comment 3

7 months ago
(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
Flags: needinfo?(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)?
Flags: needinfo?(twisniewski) → needinfo?(erahm)

Comment 5

7 months ago
(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
Flags: needinfo?(erahm)
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.
See Also: → 1483478
It's probably worth noting that you really need to apply mccr8's patch for the heap dumps to be useful...
Moving this into the Firefox for Android product so we can properly track this regression.
Product: Web Compatibility Tools → Firefox for Android
Any updates here?
Flags: needinfo?(twisniewski)
Assignee: nobody → twisniewski
Priority: -- → P2
No updates yet, I'm afraid. I'll investigate this ASAP.
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.
ni? for Comment #11
Flags: needinfo?(lhenry)
Looks like we are just tracking for beta 65. I will leave that up to Ryan.
Flags: needinfo?(lhenry)
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?
Flags: needinfo?(ryanvm)
I'll flip it to disabled after 65 is merged to release in a couple weeks.
Flags: needinfo?(ryanvm)

Thomas are you working on this for 66 nightly?

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?

Flags: needinfo?(twisniewski)
Assignee: twisniewski → nobody

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

Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.