When OOM the keyboard App should have lower priority than the foreground App

RESOLVED FIXED in Firefox 26

Status

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: kanru, Assigned: kanru)

Tracking

unspecified
1.2 C2(Oct11)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:koi+, firefox25 wontfix, firefox26 fixed, firefox27 fixed, b2g-v1.2 fixed)

Details

(Whiteboard: QARegressExclude)

Attachments

(1 attachment, 2 obsolete attachments)

I think the role=keyboard app should have lower priority than the foreground app that triggers it but higher priority than the rest of other apps. So we try to keep the keyboard open as hard as possible but don't let it kill the foreground app.
Controlled by dom/ipc/ProcessPriorityManager.cpp
Hmm.. should we reuse the BACKGROUND_PERCEIVABLE class or create a new BACKGROUND_KEYBOARD (or FOREGROUND?)

Probably FOREGROUND_KEYBOARD when the keyboard is activated and BACKGROUND when it hides.
1. We can only WRITE 6 pairs of KillUnderMB/OomScoreAdjust to sys/module/lowmemorykiller/parameters/. We are currently running out of these pairs.

Since OOM_ADJUST_MAX = 15 (or OOM_SCORE_ADJ_MAX = 1000 in later kernel),
We can enlarge these pairs' OomScoreAdjust. But in my experience, I suggest we keep hal.processPriorityManager.gonk.BACKGROUND.OomScoreAdjust no larger than 400+267. We still need spaces for background LRU, which are bug 822325 and bug 913893. I will open another bug for enlarging OomScoreAdjusts later.

2. We can still define more LMK levels, write to each process but not write to kernel config by GonkHal. 

Take Unagi as an example, we now write 0,1,2,3,4,6 to /sys/module/lowmemorykiller/parameters/adj. We cannot write 0,1,2,3,4,5,6 to it, but we can still set 5 to a process' /proc/[pid]/oom_adj.

If we enlarge the series 0,1,2,3,4,6 to 0,1,2,6,8,10 or something else, we can have more benefit.
Depends on: 914728
Hi Kan-ru,

If you need to add new ProcessPriority, the attachment is for what we should do now. I think you may need to wait after bug 914728 land.
(In reply to Alan Huang [:ahuang] from comment #4)
> Created attachment 802824 [details] [diff] [review]
> an example to add new ProcessPriority
> 
> Hi Kan-ru,
> 
> If you need to add new ProcessPriority, the attachment is for what we should
> do now. I think you may need to wait after bug 914728 land.

Thanks, Alan. This example is really helpful.
Depends on: 912340
Assignee: nobody → kchen
keyboard_manager.js will have to set mozapptype=keyboard for the keyboard iframe.
Attachment #802824 - Attachment is obsolete: true
Attachment #802969 - Attachment is obsolete: true
Attachment #809074 - Flags: review?(fabrice)
Attachment #809074 - Flags: feedback?(ahuang)
No longer depends on: 912340
Comment on attachment 809074 [details] [diff] [review]
Assign higher priority for mozapptype=inputmethod processes

Review of attachment 809074 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/app/b2g.js
@@ +624,5 @@
>  pref("hal.processPriorityManager.gonk.FOREGROUND.KillUnderMB", 6);
>  pref("hal.processPriorityManager.gonk.FOREGROUND.Nice", 1);
>  
> +pref("hal.processPriorityManager.gonk.FOREGROUND_KEYBOARD.OomScoreAdjust", 200);
> +pref("hal.processPriorityManager.gonk.FOREGROUND_KEYBOARD.Nice", 1);

do we have a default value for the KillUnderMB pref?
Attachment #809074 - Flags: review?(fabrice) → review+
Attachment #809074 - Flags: feedback?(ahuang) → feedback+
(In reply to Fabrice Desré [:fabrice] from comment #8)
> > +pref("hal.processPriorityManager.gonk.FOREGROUND_KEYBOARD.OomScoreAdjust", 200);
> > +pref("hal.processPriorityManager.gonk.FOREGROUND_KEYBOARD.Nice", 1);
> 
> do we have a default value for the KillUnderMB pref?
No, we don't and we cannot assign any more KillUnderMBs. The android kernel can only accept 6 (OomScoreAdjust, KillUnderMB) pairs. 

But it is okay, kernel will still kill processes with larger OomScoreAdjust first, even its OomScoreAdjust don't have corresponding KillUnderMB. The (OomScoreAdjust, KillUnderMB) pairs are raising critiria for LMK.
(In reply to Alan Huang [:ahuang] from comment #9)
> (In reply to Fabrice Desré [:fabrice] from comment #8)
> > > +pref("hal.processPriorityManager.gonk.FOREGROUND_KEYBOARD.OomScoreAdjust", 200);
> > > +pref("hal.processPriorityManager.gonk.FOREGROUND_KEYBOARD.Nice", 1);
> > 
> > do we have a default value for the KillUnderMB pref?
> No, we don't and we cannot assign any more KillUnderMBs. The android kernel
> can only accept 6 (OomScoreAdjust, KillUnderMB) pairs. 
> 
> But it is okay, kernel will still kill processes with larger OomScoreAdjust
> first, even its OomScoreAdjust don't have corresponding KillUnderMB. The
> (OomScoreAdjust, KillUnderMB) pairs are raising critiria for LMK.

Thanks for explanation. I'll add them to the comments in b2g.js
https://hg.mozilla.org/mozilla-central/rev/b8f0eace02bc
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
This is the blocker for OOP which is needed for new keyboard framework. So nominate it to koi+
blocking-b2g: --- → koi+
Keywords: checkin-needed
Whiteboard: QARegressExclude
Blocks: 928270
You need to log in before you can comment on or make changes to this bug.