Kan-Ru Chen [:kanru] (UTC+9) Assignee
	Description • 11 years ago

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.

	Kan-Ru Chen [:kanru] (UTC+9) Assignee
	Comment 1 • 11 years ago

Controlled by dom/ipc/ProcessPriorityManager.cpp

	Kan-Ru Chen [:kanru] (UTC+9) Assignee
	Comment 2 • 11 years ago

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.

	Alan Huang
	Comment 3 • 11 years ago

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.

	Alan Huang
	Comment 4 • 11 years ago

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.

	Kan-Ru Chen [:kanru] (UTC+9) Assignee
	Comment 5 • 11 years ago

(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.

	Kan-Ru Chen [:kanru] (UTC+9) Assignee
	Comment 7 • 11 years ago

keyboard_manager.js will have to set mozapptype=keyboard for the keyboard iframe.

	[:fabrice] Fabrice Desré
	Comment 8 • 11 years ago

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?

	Alan Huang
	Comment 9 • 11 years ago

(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.

	Kan-Ru Chen [:kanru] (UTC+9) Assignee
	Comment 10 • 11 years ago

(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

	Kan-Ru Chen [:kanru] (UTC+9) Assignee
	Comment 11 • 11 years ago

https://hg.mozilla.org/integration/b2g-inbound/rev/b8f0eace02bc

	Ryan VanderMeulen [:RyanVM]
	Comment 12 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/b8f0eace02bc

	Ivan Tsay (:ITsay)
	Comment 13 • 11 years ago

This is the blocker for OOP which is needed for new keyboard framework. So nominate it to koi+

	Ryan VanderMeulen [:RyanVM]
	Comment 14 • 11 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/00c23047048a