Closed Bug 1028247 Opened 10 years ago Closed 10 years ago

Keyboard app is running always in FFOS 2.0

Categories

(Firefox OS Graveyard :: Gaia::System::Input Mgmt, defect, P1)

Product:
Firefox OS Graveyard
Component:
Gaia::System::Input Mgmt
Platform:
ARM
Gonk (Firefox OS)
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED DUPLICATE of bug 1036577
Milestone:
2.0 S6 (18july)

People

(Reporter: tkundu, Assigned: timdream)

References

Details

(Keywords: memory-footprint, perf, Whiteboard: [caf priority: p2][MemShrink:P1][c=memory p= s= u=2.0] [CR 690200]) 

In FFOS 2.0, there is a new app as below in |b2g-procrank| and it is running always:

Built-in Keyboa 964 63176K 20864K 8611K 5612K /system/b2g/plugin-container

This means that we are loosing at least 5MB more memory in v2.0 than v1.3.

Moreover, Built-in Keyboard app grows to 7MB if we launch multiple apps even without doing any keyboard activity.

This can become a bottleneck for 256MB device in FFOS 2.0 .
blocking-b2g: --- → 2.0?
blocking-b2g: 2.0? → 2.0+
Keywords: footprint, perf
Whiteboard: [MemShrink]
Hi Tim, assign to you for now, thanks.
Assignee: nobody → timdream
Target Milestone: --- → 2.0 S5 (4july)
What kind of user impact does this bug actually represents? 

(In reply to Tapas Kumar Kundu from comment #0)
> In FFOS 2.0, there is a new app as below in |b2g-procrank| and it is running
> always:
> 
> Built-in Keyboa 964 63176K 20864K 8611K 5612K /system/b2g/plugin-container
> 
> This means that we are loosing at least 5MB more memory in v2.0 than v1.3.
> 

An oop'd keyboard will get killed just like any other oop'd app. I am not entirely the impact of the above statement.

> Moreover, Built-in Keyboard app grows to 7MB if we launch multiple apps even
> without doing any keyboard activity.

This is interesting. I don't think keyboard app eats more memory when it sits idle in the background. If that's true we got a memory leak. I suspect you are seeing keyboard app loads it's dictionary and stuff while you launch apps in the foreground.

> This can become a bottleneck for 256MB device in FFOS 2.0 .

Running Built-in keyboard app out-of-process has been planned for a long time. Turning off that means we will not be able to run 3rd-party keyboards on 256MB phones. I recommend one of the following approach if you have a hard numeral memory requirement:

1. We turn off pre-launch of keyboard app when phone starts. That means slower keyboard when the user first use it but it will meet your memory requirement if we are only talking about start-up.
2. We turn off oop for built-in keyboard ONLY. That saves 5MB process overhead and return things to 1.3 (it also means keyboard will lives in b2g process and forever without being killed)
3. We turn off oop keyboard everywhere. That essentially means we are removing feature from 256MB phones.
Component: Gaia::Keyboard → Gaia::System::Input Mgmt
Flags: needinfo?(tkundu)
Whiteboard: [MemShrink] → [MemShrink:P1]
Is it reasonable to expect that if you don't have a third party keyboard installed you shouldn't have to pay the memory overhead of running the keyboard out of process?
Flags: needinfo?(timdream)
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #3)
> Is it reasonable to expect that if you don't have a third party keyboard
> installed you shouldn't have to pay the memory overhead of running the
> keyboard out of process?

I don't know. Maybe, I vaguely member there was a bug arguing that keyboard frame should be gone when it's not being used and I dup it of making all keyboards out-of-process.

Are you suggesting we should take approach (2) as a fix to this bug?
Flags: needinfo?(timdream)
Whiteboard: [MemShrink:P1] → [MemShrink:P1][c=memory p= s= u=]
Whiteboard: [MemShrink:P1][c=memory p= s= u=] → [MemShrink:P1][c=memory p= s= u=2.0]
Output of |adb reboot && sleep 60 && adb shell b2g-procrank && adb shell procrank && adb shell cat /proc/meminfo| 

In FFOS 1.3 : 

 
APPLICATION        PID      Vss      Rss      Pss      Uss  cmdline
b2g                293   83368K   72000K   59637K   52908K  /system/b2g/b2g
Homescreen         949   32460K   30008K   19046K   13596K  /system/b2g/plugin-container
(Nuwa)             928   21540K   21412K    9989K    4368K  /system/b2g/plugin-container
(Preallocated a   1091   18724K   18596K    9326K    5264K  /system/b2g/plugin-container
                                           ------   ------  ------


In FFOS 2.0 : 

APPLICATION        PID       Vss      Rss      Pss      Uss  cmdline
b2g                214   240536K   79128K   62758K   53500K  /system/b2g/b2g
Homescreen        1062   118940K   43100K   27952K   20852K  /system/b2g/plugin-container
Built-in Keyboa   1023    71108K   21552K   10362K    6816K  /system/b2g/plugin-container
(Preallocated a   1102    60108K   18752K    8560K    5492K  /system/b2g/plugin-container
(Nuwa)             951    53884K   20324K    7541K    2300K  /system/b2g/plugin-container


(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #2)
>
> 1. We turn off pre-launch of keyboard app when phone starts. That means
> slower keyboard when the user first use it but it will meet your memory
> requirement if we are only talking about start-up.

It seems like a trade off between performance vs memory for keyboard app launch. I am OK with it because keyboard app is killed by LMK if additional memory is needed.

> 2. We turn off oop for built-in keyboard ONLY. That saves 5MB process
> overhead and return things to 1.3 (it also means keyboard will lives in b2g
> process and forever without being killed)

We are seeing that b2g process USS memory is not saved although keyboard app is removed from b2g process in FFOS 2.0. Does it mean that b2g process has grown up between FFOS 1.3 to FFOS 2.0 ? Please suggest.
Flags: needinfo?(tkundu) → needinfo?(timdream)
(In reply to Tapas Kumar Kundu from comment #5)
> Output of |adb reboot && sleep 60 && adb shell b2g-procrank && adb shell
> procrank && adb shell cat /proc/meminfo| 
> 
> In FFOS 1.3 : 
> 
>  
> APPLICATION        PID      Vss      Rss      Pss      Uss  cmdline
> b2g                293   83368K   72000K   59637K   52908K  /system/b2g/b2g
> Homescreen         949   32460K   30008K   19046K   13596K 
> /system/b2g/plugin-container
> (Nuwa)             928   21540K   21412K    9989K    4368K 
> /system/b2g/plugin-container
> (Preallocated a   1091   18724K   18596K    9326K    5264K 
> /system/b2g/plugin-container
>                                            ------   ------  ------
> 
> 
> In FFOS 2.0 : 
> 
> APPLICATION        PID       Vss      Rss      Pss      Uss  cmdline
> b2g                214   240536K   79128K   62758K   53500K  /system/b2g/b2g
> Homescreen        1062   118940K   43100K   27952K   20852K 
> /system/b2g/plugin-container
> Built-in Keyboa   1023    71108K   21552K   10362K    6816K 
> /system/b2g/plugin-container
> (Preallocated a   1102    60108K   18752K    8560K    5492K 
> /system/b2g/plugin-container
> (Nuwa)             951    53884K   20324K    7541K    2300K 
> /system/b2g/plugin-container
> 
> 
> (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from
> comment #2)
> >
> > 1. We turn off pre-launch of keyboard app when phone starts. That means
> > slower keyboard when the user first use it but it will meet your memory
> > requirement if we are only talking about start-up.
> 
> It seems like a trade off between performance vs memory for keyboard app
> launch. I am OK with it because keyboard app is killed by LMK if additional
> memory is needed.
> 

The "I am OK" part and the following "because" doesn't line up. An oop'd keyboard app will be killed regardless of doing (1) or not. If not doing anything (not even (1) as stated) fits your requirement this is a WONTFIX bug.

> > 2. We turn off oop for built-in keyboard ONLY. That saves 5MB process
> > overhead and return things to 1.3 (it also means keyboard will lives in b2g
> > process and forever without being killed)
> 
> We are seeing that b2g process USS memory is not saved although keyboard app
> is removed from b2g process in FFOS 2.0. Does it mean that b2g process has
> grown up between FFOS 1.3 to FFOS 2.0 ? Please suggest.

Possibility, I don't know.
Flags: needinfo?(timdream)
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #6)

> > > 2. We turn off oop for built-in keyboard ONLY. That saves 5MB process
> > > overhead and return things to 1.3 (it also means keyboard will lives in b2g
> > > process and forever without being killed)
> > 
> > We are seeing that b2g process USS memory is not saved although keyboard app
> > is removed from b2g process in FFOS 2.0. Does it mean that b2g process has
> > grown up between FFOS 1.3 to FFOS 2.0 ? Please suggest.
> 
> Possibility, I don't know.

Could you please post test patch/steps to do (2). I want to see whether b2g process memory consumption increases with it or not. I also want to know whether we will see any delay in keyboard startup if we do this or not.
Flags: needinfo?(timdream)
(2) need to be written, I have no patch readily available for you. However since you only concern about built-in keyboard app, you can run your test by turning off oop for keyboard first. Just revert bug 982568 locally to try it.

|git revert -m 1 db2ef2b61c70889533a0837fa3e053d24e95fdea|

Thanks!
Flags: needinfo?(timdream)
Tapas,

I still need your opinion to tell what counts as a fix to this bug, so I am keeping the needinfo to you.
Flags: needinfo?(tkundu)
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #9)
> Tapas,
> 
> I still need your opinion to tell what counts as a fix to this bug, so I am
> keeping the needinfo to you.

Sorry for late reply. There is a tradeoff for this usecase.

Consider a usecase: (A) Launch browser and typing with keyboard

If we compare two builds
1) With patch from #comment 8 :
b2g is taking 59972KB(USS) memory and b2g process is displaying keyboard itself.

2) Without patch from #comment 8 i.e normal FFOS 2.0
Cons: 
1) b2g is taking 56248KB (USS) but keyboard app is running separately and it is raking 7064KB . So Total usage is 63MB which is 3MB higher than build-1. 

2) Consider that user is running camera, gallery, setting etc on 256MB device. If we launch keyboard then it will kill background app quickly than v1.3 because we will be using 3MB extra memory.

Pros:
1) But keyboard app can be killed by LMK if we put keyboard app in background i.e not using keyboard for a long time. And this will save 5MB more memory than v1.3


If we can take advantage from both build-1 and build-2 then it will be best for low memory target. 

So solution can be :
1) Currently keyboard app is running separately and it is taking ~7MB (USS) which is causing device to have 3MB less memory than v1.3 and it can lead to killing background app more frequently in v2.0 when keyboard runs. 

If we can optimize keyboard app more so that total memory usage from usecase (A) will be as good as FFOS v1.3 then it will be best solution for v2.0.
Flags: needinfo?(timdream)
Flags: needinfo?(tkundu)
(In reply to Tapas Kumar Kundu from comment #10)

Thanks for the feedback. Let me put your numbers into a table:

        B2G         oop Keyboard        Total
---------------------------------------------
inproc  59972KB      0                59972KB
oop     56248KB      7064KB           63312KB
---------------------------------------------
delta   -3724KB     +7064KB           +3340KB

> So solution can be :
> 1) Currently keyboard app is running separately and it is taking ~7MB (USS)
> which is causing device to have 3MB less memory than v1.3 and it can lead to
> killing background app more frequently in v2.0 when keyboard runs. 
> 
> If we can optimize keyboard app more so that total memory usage from usecase
> (A) will be as good as FFOS v1.3 then it will be best solution for v2.0.

I don't think the "best solution" here is possible. From our experience, there isn't much things to do in the application JavaScript context to reduce memory usage, and small wins will never achieve what you want, especially in the 2.0+ time frame ... unless we found some dump memory pattern in the app like loading the database/big images twice for no reason.

Do you have any second best solution?
Flags: needinfo?(timdream) → needinfo?(tkundu)
Flags: needinfo?(tkundu)
Flags: needinfo?(mvines)
Flags: needinfo?(mvikram)
@mvines
Can you please suggest whether we should try to optimize for 3MB extra memory usage for following usecase [1] in FFOS 2.0 timeframe ? . This usecase [1] (or any keyboard usecase) is using 3MB more memory in FFOS 2.0 than FFOS 1.3. 

This usecase (or any keyboard usecase) may cause background apps to be killed more frequently than FFOS v1.3 . 


1) Typing URL in browser using keyboard.
Why isn't moving the built-in keyboard back in process an acceptable solution to this bug?
Whiteboard: [MemShrink:P1][c=memory p= s= u=2.0] → [MemShrink:P1][c=memory p= s= u=2.0] [CR 1028253]
Whiteboard: [MemShrink:P1][c=memory p= s= u=2.0] [CR 1028253] → [MemShrink:P1][c=memory p= s= u=2.0] [CR 690200]
Whiteboard: [MemShrink:P1][c=memory p= s= u=2.0] [CR 690200] → [caf priority: p1][MemShrink:P1][c=memory p= s= u=2.0] [CR 690200]
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #13)
> Why isn't moving the built-in keyboard back in process an acceptable
> solution to this bug?

It might, I would just like to confirm with codeaurora.org before start writing the patch. Kyle, is that unnecessary for you?
Flags: needinfo?(khuey)
I don't think we can say for sure that this 3MB RAM increase while the keyboard is visible is actually v2.0+.  If the foreground application is < 3MB away from getting LMKed then it probably needs to shrink regardless.  Otherwise, this increase only potentially results in additional background applications getting killed which would have an impact on subsequent apps start times but that's probably okay.  

Until the much larger v2.0 memory regressions are resolved (yes Vertical Homescreen, I'm looking at you) it probably isn't worth trying to squeeze this rock too hard.  That being said moving the built-in keyboard back in process sounds real nice, perhaps as a Gaia configuration option to be enabled in the 256MB build
Flags: needinfo?(mvines)
Setting 2.0? Based on above comment. This bug remain valid only if we want to make a special config or general config change to put built-in keyboard app in-proc.
blocking-b2g: 2.0+ → 2.0?
Whiteboard: [caf priority: p1][MemShrink:P1][c=memory p= s= u=2.0] [CR 690200] → [caf priority: p2][MemShrink:P1][c=memory p= s= u=2.0] [CR 690200]
not a blocker based on Comment 15 and Comment 16
blocking-b2g: 2.0? → ---
Is there a strong and definite need for making special config change to put built-in keyboard in-proc right now? If no, I'll put this bug as won't fix. We can just open another bug to do the config change once it's confirmed needed afterwards.
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #14)
> (In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #13)
> > Why isn't moving the built-in keyboard back in process an acceptable
> > solution to this bug?
> 
> It might, I would just like to confirm with codeaurora.org before start
> writing the patch. Kyle, is that unnecessary for you?

I still don't understand why we wouldn't do this regardless of whether they are concerned by it.  Put another way, what do we win by moving the built in keyboard out of process (and using 3MB more of memory)?
Flags: needinfo?(khuey)
When it's our own keyboard, we win nothing. That looks very sensible to only run 3rd party keyboards oop. I wonder why we didn't think about that earlier!
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #19)
> I still don't understand why we wouldn't do this regardless of whether they
> are concerned by it.  Put another way, what do we win by moving the built in
> keyboard out of process (and using 3MB more of memory)?

As comment 11 have shown, we won 3.7MB in chrome process when you are playing a game without any keyboard.

(In reply to Fabrice Desré [:fabrice] from comment #20)
> When it's our own keyboard, we win nothing. That looks very sensible to only
> run 3rd party keyboards oop. I wonder why we didn't think about that earlier!

We will be keeping the inproc built-in keyboard frame live regardless of the keyboard user is currently using in this case, so this will ended up consuming more memory. Only if the said 3rd-party keyboard implements all input types, and the user correctly disables all layouts for all types of build-in keyboard app, the user will benefited from the scenario you have thought.
Flags: needinfo?(mvikram)
No longer blocks: CAF-v2.0-FC-metabug
Any solution of this bug is essentially the same as bug 1036577 so dup them together.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
Target Milestone: 2.0 S5 (4july) → 2.0 S6 (18july)
You need to log in before you can comment on or make changes to this bug.