Closed
Bug 785960
Opened 12 years ago
Closed 12 years ago
"Don't keep activities" developer option gets counted as OOM in telemetry
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 19
People
(Reporter: kats, Assigned: gcp)
References
Details
Attachments
(2 files)
1.30 KB,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
4.18 KB,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
Bug 779687 added telemetry to track OOM kills. However this code also gets run whenever the user dismisses the awesomebar when they have the "don't keep activities" developer option set. It would be good to detect when this option is set and not count that as an OOM kill.
Assignee | ||
Comment 1•12 years ago
|
||
As far as I can tell this avoids the bug and still triggers in all the real OOM cases (in those Gecko also gets killed).
Assignee: nobody → gpascutto
Attachment #669175 -
Flags: review?(bugmail.mozilla)
Assignee | ||
Comment 2•12 years ago
|
||
We're adding FENNEC_ to all our new telemetry, so now that we likely will start analysis on this one afresh, use the opportunity to rename it. Add a flag to see if the user is running with "Dont keep activities" enabled. Might come in useful for performance analysis.
Attachment #669176 -
Flags: review?(bugmail.mozilla)
Reporter | ||
Comment 3•12 years ago
|
||
Comment on attachment 669175 [details] [diff] [review] Patch 1. Don't count OOM when restoring activities Review of attachment 669175 [details] [diff] [review]: ----------------------------------------------------------------- LGTM
Attachment #669175 -
Flags: review?(bugmail.mozilla) → review+
Reporter | ||
Comment 4•12 years ago
|
||
Comment on attachment 669176 [details] [diff] [review] Patch 2. Rework telemetry name, add for "dont keep activities" Review of attachment 669176 [details] [diff] [review]: ----------------------------------------------------------------- To me the name "FENNEC_WAS_KILLED" implies that the histogram will count all instances of fennec being killed, including e.g. if somebody runs "kill" from adb shell or swipes away the app from the JB app switcher. However the histogram doesn't actually include those instances. I would prefer if this were renamed to "FENNEC_OOM_KILLED" since that's really what we're measuring here, AFAICT.
Assignee | ||
Comment 5•12 years ago
|
||
>To me the name "FENNEC_WAS_KILLED" implies that the histogram will count all
>instances of fennec being killed, including e.g. if somebody runs "kill" from adb
>shell or swipes away the app from the JB app switcher. However the histogram
>doesn't actually include those instances.
It *will* count a "kill" from the adb shell or a kill from the Task Manager if the application was in the foreground when it happened. That's why I renamed it - there are a few cases that are indistinguishable from an OOM kill.
The most descriptive name would probably be FENNEC_WAS_KILLED_IN_FOREGROUND.
Reporter | ||
Comment 6•12 years ago
|
||
Comment on attachment 669176 [details] [diff] [review] Patch 2. Rework telemetry name, add for "dont keep activities" Review of attachment 669176 [details] [diff] [review]: ----------------------------------------------------------------- Ah hmm. However it will also count a kill from an OutOfMemoryError thrown while it is in the background, right? The uncaught exception handler sets the GeckoApp.PREFS_OOM_EXCEPTION shared pref which will increment the histogram on the next run. Maybe FENNEC_WAS_KILLED is good enough.
Attachment #669176 -
Flags: review?(bugmail.mozilla) → review+
Assignee | ||
Comment 7•12 years ago
|
||
>However it will also count a kill from an OutOfMemoryError thrown while it is in
>the background, right?
It's going to be harder to OOM in Java code while the activity is suspended :-)
Assignee | ||
Comment 8•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c0b4572696b6 https://hg.mozilla.org/integration/mozilla-inbound/rev/c80a02b9d478
Reporter | ||
Comment 9•12 years ago
|
||
(In reply to Gian-Carlo Pascutto (:gcp) from comment #7) > It's going to be harder to OOM in Java code while the activity is suspended > :-) Hard, but not impossible. :) Gecko could still be running stuff and sending messages to Java while the activity is suspended.
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c0b4572696b6 https://hg.mozilla.org/mozilla-central/rev/c80a02b9d478
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•