Closed Bug 785960 Opened 7 years ago Closed 7 years ago

"Don't keep activities" developer option gets counted as OOM in telemetry

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 19

People

(Reporter: kats, Assigned: gcp)

References

Details

Attachments

(2 files)

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.
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)
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)
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+
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.
>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.
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+
>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 :-)
(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.
https://hg.mozilla.org/mozilla-central/rev/c0b4572696b6
https://hg.mozilla.org/mozilla-central/rev/c80a02b9d478
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
You need to log in before you can comment on or make changes to this bug.