1039171 - Decide keyboard inproc/oop based on total memory of the device

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Keyboard, defect)

Description

[Tim Guan-tin Chien [:timdream] (please needinfo)]

In bug 1028247 and bug 970188 it was concluded that there will not be any user-facing impact moving built-in keyboard out-of-process, however in bug 1036577 a rather late requirement (keeping the support of "273MB" Flame) revert that conclusion and move the app inproc again. 

At this point, it is clear that we need a runtime API available to System app to tell input management whether or not it should launch the preload keyboard apps OOP. I am filing this bug because bug 1036577 (moving all preload keyboard apps inproc) is not a complete solution and obviously a memory regression to hi-end phones, although it makes 273MB Flame work and make QA people happy.

There is a built time flag called GAIA_MEMORY_PROFILE, put in during Tarako work. I can use that and insert a built time flag into System app, but it seems that it will not fit the test methodology used by 273MB Flame testing (QA people are essentially using the same image but tweak the avail memory in fastboot oem config).

All of this lead to lot of confusion and frustration, on top of our already depleted attention span to work with multiple partners and configurations. Can someone drive proper fix like this one instead of asking Gaia people running around?

Comment 1

[[:fabrice] Fabrice Desré]

I said that already in another bug (one the camera or gallery related one), that API exists, it's navigator.getFeature("hardware.memory").

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Fabrice Desré [:fabrice] from comment #1)
> I said that already in another bug (one the camera or gallery related one),
> that API exists, it's navigator.getFeature("hardware.memory").

Looks good. Although bug 1009645 will not be landed to 2.0 so we will still have implement a switch in bug 1036577.

Still that means input management need some magic numbers to work with what's returned from this API, something like

if (memory < 256) {
  // no oop keyboards and no 3rd-party keyboards
} else if (memory <= 273) {
  // inproc preload keyboards and oop 3rd-party keyboards
} else {
  // oop every keyboards
}

Comment 3

[Tim Guan-tin Chien [:timdream] (please needinfo)]

It looks like bug 1037097 already do what comment 2 said.

https://github.com/mozilla-b2g/gaia/blob/24ada16dff049c657f37518f79eb964c491e64c6/shared/js/media/media_frame.js#L55-L81

Given the fact, I presume comment 2 is a viable pattern and I will implement that on top of bug 1036577.

Comment 4

[Mozilla Shepherd (This account is used for automation and doesn't receive bugmail)]

Attachment: mozilla-b2g:master PR#21907

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8458448 [details] [review]
mozilla-b2g:master PR#21907

Follow-up to bug 1036577.

Comment 6

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Comment on attachment 8458448 [details] [review]
mozilla-b2g:master PR#21907

r=me with a suggestion to define the threshold (512 MB) as a settings value,
https://github.com/mozilla-b2g/gaia/pull/21907#discussion_r15099030

Thank you.

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Rudy Lu [:rudyl] from comment #6)
> r=me with a suggestion to define the threshold (512 MB) as a settings value,
> https://github.com/mozilla-b2g/gaia/pull/21907#discussion_r15099030

I don't see the use case to put this number in Settings database.

master: https://github.com/mozilla-b2g/gaia/commit/d525416a1a644411d3de1474260c6b080aff5b16

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8458448 [details] [review]
mozilla-b2g:master PR#21907

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 1036577
[User impact] if declined: The behavior of 2.0 and 2.1 will be different -- on hi-end phones 2.0 will keep keyboard app inproc and waste memory
[Testing completed]: manually tests on the phone
[Risk to taking this patch] (and alternatives if risky): patch should be small enough for uplift
[String changes made]: none

Comment 9

[(no longer active)]

Note that getFeature() returns a number rounded up to the next power of two.  Not sure if that matters for this bug though.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/98bca47c14bfa93afb7fd746445cfddaf98a7522