[tarako only] Run the homescreen in process

RESOLVED FIXED

Status

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: fabrice, Assigned: fabrice)

Tracking

unspecified
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:1.3T+, b2g-v1.3T fixed)

Details

(Whiteboard: [sprd295070][tarako_only])

Attachments

(2 attachments, 1 obsolete attachment)

Yes, it's sad. But that gives us a better user experience overall from my testing.
Posted patch homescreen-inproc.patch (obsolete) — Splinter Review
Attachment #8426604 - Flags: review?(alive)
If it seems better do we want to block or backlog this?
Flags: needinfo?(fabrice)
Oh, I want to land asap, let partners and QA do a test run and see where we go from there.
Flags: needinfo?(fabrice)
Setting to backlog then.
blocking-b2g: --- → backlog
homescreen and music will run in process, is there any side effect?
I think 3rd party app will be worse.
Can gecko kill in process app when low memory pressure?
(In reply to James Zhang from comment #5)
> homescreen and music will run in process, is there any side effect?
> I think 3rd party app will be worse.

James, look at the patch, it's a one line change. With that I get better results with games/facebook going into background. That needs more testing for sure.
(In reply to James Zhang from comment #6)
> Can gecko kill in process app when low memory pressure?

We don't really need to kill the app. What helps is that we don't have to restart the homescreen - that is putting memory pressure that lead to killing background apps.
Comment on attachment 8426604 [details] [diff] [review]
homescreen-inproc.patch

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

This makes every frame in proc. You should add one more rule here: https://github.com/mozilla-b2g/gaia/blob/v1.3t/apps/system/js/browser_config_helper.js#L86
Attachment #8426604 - Flags: review?(alive) → review-
I disagree... I did the browser_config_helper.js#L86 and that didn't work. With the attached patch only the homescreen (and currently the keyboard) is in process on tarako, all the other apps are oop, including browser tabs.
(In reply to Fabrice Desré [:fabrice] from comment #10)
> I disagree... I did the browser_config_helper.js#L86 and that didn't work.
> With the attached patch only the homescreen (and currently the keyboard) is
> in process on tarako, all the other apps are oop, including browser tabs.

The root cause is we have duplicate codes in old v1.3 window_manager to decide oop or not. This is bad.
https://github.com/mozilla-b2g/gaia/blob/v1.3t/apps/system/js/window_manager.js#L797

Could you update the patch ?
Whiteboard: [sprd295070]
(In reply to Fabrice Desré [:fabrice] from comment #7)
> (In reply to James Zhang from comment #5)
> > homescreen and music will run in process, is there any side effect?
> > I think 3rd party app will be worse.
> 
> James, look at the patch, it's a one line change. With that I get better
> results with games/facebook going into background. That needs more testing
> for sure.

Yang, please land WIP patch on my side and run monkey test to verify.
Fabrice, you need get review+ first.
blocking-b2g: backlog → 1.3T?
Flags: needinfo?(yang.zhao)
Yep, not sure what I did wrong the first time, adding the homescreen to the list just works.
Attachment #8426604 - Attachment is obsolete: true
Attachment #8426826 - Flags: review?(alive)
Attachment #8426826 - Flags: review?(alive) → review+
Fabrice, I suggest that we can move blacklist to setting.json. We can't land this patch to v2.0 for tarako.
(In reply to James Zhang from comment #15)
> Fabrice, I suggest that we can move blacklist to setting.json. We can't land
> this patch to v2.0 for tarako.

It doesn't make sense to create settings toward device. Where is this determined? Is there any strategy decided?
See comment14,Alive already merge it into v1.3t
Flags: needinfo?(yang.zhao)
Depends on: 1014704
We met onece monkey test crash after home screen disable oop.
libxul.so!mozilla::dom::TabChild::RecvRealTouchEvent(mozilla::WidgetTouchEvent const&, mozilla::layers::ScrollableLayerGuid const&) [nsCOMPtr.h : 554 + 0x2]
     r4 = 0x42840880    r5 = 0xbec6a808    r6 = 0x41ecb92c    r7 = 0xbec6a740
     r8 = 0x00000001    r9 = 0xbec6a8b8   r10 = 0xbec6a7b8    fp = 0xbec6a7a8
     sp = 0xbec6a738    lr = 0x409f1b91    pc = 0x40e9834e
    Found by: given as instruction pointer in context
 1  libxul.so!mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&) [PBrowserChild.cpp : 2131 + 0xd]
     r4 = 0x42840880    r5 = 0xbec6aed4    r6 = 0xbec6a808    r7 = 0xbec6a8b8
     r8 = 0xbec6a970    r9 = 0x00000000   r10 = 0x00000003    fp = 0x00000000
     sp = 0xbec6a7e8    pc = 0x40b2b315
    Found by: call frame info
 2  libxul.so!mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) [PContentChild.cpp : 3170 + 0x7]
     r4 = 0x4318d818    r5 = 0xbec6aed4    r6 = 0xbec6aef8    r7 = 0x41ecb92c
     r8 = 0x00000006    r9 = 0x4031e90c   r10 = 0x00000003    fp = 0x00000000
     sp = 0xbec6aa20    pc = 0x40b44339
    Found by: call frame info
(In reply to James Zhang from comment #18)
> We met onece monkey test crash after home screen disable oop.
> libxul.so!mozilla::dom::TabChild::RecvRealTouchEvent(mozilla::
> WidgetTouchEvent const&, mozilla::layers::ScrollableLayerGuid const&)
> [nsCOMPtr.h : 554 + 0x2]
>      r4 = 0x42840880    r5 = 0xbec6a808    r6 = 0x41ecb92c    r7 = 0xbec6a740
>      r8 = 0x00000001    r9 = 0xbec6a8b8   r10 = 0xbec6a7b8    fp = 0xbec6a7a8
>      sp = 0xbec6a738    lr = 0x409f1b91    pc = 0x40e9834e
>     Found by: given as instruction pointer in context
>  1  libxul.so!mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message
> const&) [PBrowserChild.cpp : 2131 + 0xd]
>      r4 = 0x42840880    r5 = 0xbec6aed4    r6 = 0xbec6a808    r7 = 0xbec6a8b8
>      r8 = 0xbec6a970    r9 = 0x00000000   r10 = 0x00000003    fp = 0x00000000
>      sp = 0xbec6a7e8    pc = 0x40b2b315
>     Found by: call frame info
>  2  libxul.so!mozilla::dom::PContentChild::OnMessageReceived(IPC::Message
> const&) [PContentChild.cpp : 3170 + 0x7]
>      r4 = 0x4318d818    r5 = 0xbec6aed4    r6 = 0xbec6aef8    r7 = 0x41ecb92c
>      r8 = 0x00000006    r9 = 0x4031e90c   r10 = 0x00000003    fp = 0x00000000
>      sp = 0xbec6aa20    pc = 0x40b44339
>     Found by: call frame info

see bug 1004266.
Depends on: 1004266
Triage: no need for v1.4.
We found we can't set wall paper after monkey test. Restart or kill home screen, it's OK.
Yang, please comment here.
Flags: needinfo?(yang.zhao)
I found the following error like the followings:

./main.log:05-26 14:52:16.420   609   609 E GeckoConsole: [JavaScript Error: "NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIMessageSender.sendAsyncMessage]" {file: "resource://gre/modules/ActivitiesService.jsm" line: 15}]
./main.log:05-26 14:54:22.100   609   609 E GeckoConsole: [JavaScript Error: "NS_ERROR_NOT_INITIALIZED: Component returned failure code: 0xc1f30001 (NS_ERROR_NOT_INITIALIZED) [nsIMessageSender.sendAsyncMessage]" {file: "resource://gre/modules/ActivitiesService.jsm" line: 14}]


I don't know  whether the log is related to this issue.
After running monkey test ,the background is black ,but when I pull the log out from the sdcard,and wait for a while ,the background recovers to normal.Long press on the homescreen,then select 'Change Wallpaper'->'Wallpaper' ,the phone just returned to idle which should jump to wallpaper activity to select.
Flags: needinfo?(yang.zhao)
(In reply to James Zhang from comment #21)
> We found we can't set wall paper after monkey test. Restart or kill home
> screen, it's OK.
> Yang, please comment here.

That's fixed by bug 1014704
blocking-b2g: 1.3T? → 1.3T+
Fabrice, with bug 1014704 patch, we met new bug 1016746.
Depends on: 1016746
Depends on: 1019156
refer to 
https://github.com/mozilla-b2g/gaia/pull/19160

can we do similar actions for homescreen?
when receiving memory pressure event, kill homscreen to save memory?
Flags: needinfo?(dkuo)
Flags: needinfo?(alive)
(In reply to ying.xu from comment #25)
> refer to 
> https://github.com/mozilla-b2g/gaia/pull/19160
> 
> can we do similar actions for homescreen?
> when receiving memory pressure event, kill homscreen to save memory?

Yes, we can, but we should only kill the homescreen at the right timings, like in the background. I believe Alive knows how to manage the homescreen's life cycle better than me.
Flags: needinfo?(dkuo)
Yes you could.
Flags: needinfo?(alive)
We must kill homescreen to pass top 3rd-party apps test case.
We should kill homescreen before foreground app be killed.
(In reply to James Zhang from comment #29)
> We should kill homescreen before foreground app be killed.

James, there is little difference to kill a inproc homescreen app with memorypressure event v.s. OOM an OOP homescreen app. If we do that we will regress the nice back-to-homescreen experience. If you really think we should do that we should simply back out this patch from v1.3t.

If you agree I will backout the patch.
Flags: needinfo?(james.zhang)
We need find the balance between homescreen performance and foreground app killed.
I'll check this issue with our PM and Sales.
Flags: needinfo?(james.zhang)
Correcting v2.0 flag -- no patch of this bug has ever reaches v2.0, I don't know why :bajaj change the flag.
Whiteboard: [sprd295070] → [sprd295070][tarako_only]
We found if homescreen can be killed before foreground, we can save about 8MB PSS in camera case(Facebook will use this test case).
And can we modify "hal.processPriorityManager.gonk.BACKGROUND_HOMESCREEN.KillUnderKB" to avoid homescreen be killed easily?

home screen in process

camera
APPLICATION       OOM_ADJ  OOM_SCORE  OOM_SCORE_ADJ   PID      Vss      Rss      Pss      Uss  cmdline
b2g                  0        275         0            86   50440K   45744K   37434K   33420K  /system/b2g/b2g
Camera               2        299         134         460   27860K   25596K   16585K   12088K  /system/b2g/b2g
(Preallocated a      4        346         267         693   15960K   15944K    8423K    5348K  /system/b2g/b2g
(Nuwa)               -16      1           -941        297    6760K    6760K    2042K     464K  /system/b2g/b2g
                                                                              ------   ------  ------
                                                                              68306K   54452K  TOTAL

homescreen only
APPLICATION       OOM_ADJ  OOM_SCORE  OOM_SCORE_ADJ   PID      Vss      Rss      Pss      Uss  cmdline
b2g                  0        308         0           621   52432K   49696K   45103K   41760K  /system/b2g/b2g
(Preallocated a      4        342         267         959   14628K   14612K    9929K    6496K  /system/b2g/b2g
(Nuwa)               -16      1           -941        634    3904K    3904K    1466K     232K  /system/b2g/b2g
                                                                              ------   ------  ------
                                                                              59334K   50748K  TOTAL


homescreen out of process

camera

APPLICATION       OOM_ADJ  OOM_SCORE  OOM_SCORE_ADJ   PID      Vss      Rss      Pss      Uss  cmdline
b2g                  0        242         0            86   43640K   38932K   32071K   28440K  /system/b2g/b2g
Camera               2        285         134        1470   25388K   23120K   15487K   11272K  /system/b2g/b2g
(Preallocated a      4        344         267        1506   13964K   13960K    7853K    5116K  /system/b2g/b2g
(Nuwa)               -16      1           -941        299    4476K    4476K    1326K     256K  /system/b2g/b2g
                                                                              ------   ------  ------
                                                                              60298K   48128K  TOTAL

homescreen only
APPLICATION       OOM_ADJ  OOM_SCORE  OOM_SCORE_ADJ   PID      Vss      Rss      Pss      Uss  cmdline
b2g                  0        245         0            86   42632K   38732K   32023K   28528K  /system/b2g/b2g
Homescreen           2        280         134         943   23084K   21932K   14744K   10964K  /system/b2g/b2g
(Preallocated a      4        345         267        1000   14528K   14524K    8615K    6080K  /system/b2g/b2g
(Nuwa)               -16      1           -941        299    4460K    4460K    1304K     228K  /system/b2g/b2g
                                                                              ------   ------  ------
                                                                              58877K   47524K  TOTAL
Whiteboard: [sprd295070][tarako_only] → [sprd295070]
Whiteboard: [sprd295070] → [sprd295070][tarako_only]
(In reply to James Zhang from comment #33)
> And can we modify
> "hal.processPriorityManager.gonk.BACKGROUND_HOMESCREEN.KillUnderKB" to avoid
> homescreen be killed easily?

Thinker, do you know who should answer this question?
Flags: needinfo?(tlee)
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #30)

> James, there is little difference to kill a inproc homescreen app with
> memorypressure event v.s. OOM an OOP homescreen app. If we do that we will
> regress the nice back-to-homescreen experience. If you really think we
> should do that we should simply back out this patch from v1.3t.

homescreen in b2g can run faster than OOP, even get killed, also can restore faster than OOP.
That's the benefit we want to use.
If the question is "should we make homescreen more chance of survival?" this is a trade-off of user experiences.  It seems to be UXs or PMs of users to decide significances of the response time of the homescreen and foreground apps according their studies.

If the question is "could we make homescreen to be killed less than current?", you should check how possible the homescreen being killed for the threshold of "hal.processPriorityManager.gonk.BACKGROUND_HOMESCREEN.KillUnderKB".  Do you have a number?
Flags: needinfo?(tlee)
(In reply to Thinker Li [:sinker] from comment #36)
> If the question is "should we make homescreen more chance of survival?" this
> is a trade-off of user experiences.  It seems to be UXs or PMs of users to
> decide significances of the response time of the homescreen and foreground
> apps according their studies.
> 
> If the question is "could we make homescreen to be killed less than
> current?", you should check how possible the homescreen being killed for the
> threshold of
> "hal.processPriorityManager.gonk.BACKGROUND_HOMESCREEN.KillUnderKB".  Do you
> have a number?

"hal.processPriorityManager.gonk.BACKGROUND_HOMESCREEN.KillUnderKB", current is 16MB in dev-pref.js, origin is 8MB. And it's unused if homescreen in process.

My question is that we should protect foreground app, need kill other app to avoid foreground be killed. Currently homescreen is in process, it uses about 8MB memory, we can't shrink the 8MB memory when foreground app be killed.

My suggestion is that "hal.processPriorityManager.gonk.BACKGROUND_HOMESCREEN.KillUnderKB" is 8MB, and revert this change, homescreen is out of process.
(In reply to ying.xu from comment #35)
> (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from
> comment #30)
> 
> > James, there is little difference to kill a inproc homescreen app with
> > memorypressure event v.s. OOM an OOP homescreen app. If we do that we will
> > regress the nice back-to-homescreen experience. If you really think we
> > should do that we should simply back out this patch from v1.3t.
> 
> homescreen in b2g can run faster than OOP, even get killed, also can restore
> faster than OOP.
> That's the benefit we want to use.

We have better to do measurement of potential side-effects or regressions before making a decision like this.  We should consider pros and cons, not just talk benefits.  For this very late point, I don't think we should keep such big change without sufficiently measurements and without critical reasons.
(In reply to James Zhang from comment #37)
> My suggestion is that
> "hal.processPriorityManager.gonk.BACKGROUND_HOMESCREEN.KillUnderKB" is 8MB,
> and revert this change, homescreen is out of process.

We should do judgements base on facts, not guessing.  That means we need numbers to make decisions, or we are moving around like headless flies.  For this case, we have moved around, forward and backward, for several times.  If you have numbers of how it effects the homescreen, foreground apps, and app switching, I would like to discuss the solution.
(In reply to Thinker Li [:sinker] from comment #38)
> > homescreen in b2g can run faster than OOP, even get killed, also can restore
> > faster than OOP.
> > That's the benefit we want to use.
> 
> We have better to do measurement of potential side-effects or regressions

Here is my test.

set /sys/module/lowmemorykiller/parameters/notify_trigger:  6172

settings at backgournd,  b2g took 33996K
apusr@yingxuubt:~/b2gsource/ffos/gaia$  adb shell b2g-procrank --oom
APPLICATION       OOM_ADJ  OOM_SCORE  OOM_SCORE_ADJ   PID      Vss      Rss      Pss      Uss  cmdline
b2g                  0        275         0          5582   45708K   42716K   37149K   33996K  /system/b2g/b2g
Settings             10       790         667        7320   16252K   16252K   10650K    7472K  /system/b2g/b2g
(Preallocated a      4        325         267       13144   10196K   10192K    5729K    3648K  /system/b2g/b2g
(Nuwa)               -16      1           -941       5585    2088K    2088K     629K     140K  /system/b2g/b2g
                                                                              ------   ------  ------
                                                                              56102K   46740K  TOTAL


settings at foreground, killing homscreen, b2g took 29608K
apusr@yingxuubt:~/b2gsource/ffos/gaia$  adb shell b2g-procrank --oom
APPLICATION       OOM_ADJ  OOM_SCORE  OOM_SCORE_ADJ   PID      Vss      Rss      Pss      Uss  cmdline
b2g                  0        258         0          5582   41604K   38036K   32599K   29608K  /system/b2g/b2g
Settings             2        256         134        7320   15504K   14928K    9470K    6472K  /system/b2g/b2g
(Preallocated a      4        326         267       13144   10696K   10692K    6115K    3960K  /system/b2g/b2g
(Nuwa)               -16      1           -941       5585    2108K    2108K     646K     156K  /system/b2g/b2g
                                                                              ------   ------  ------
                                                                              50598K   41508K  TOTAL

the speed of homescreen restoring at this case was about 1s, no more than 1.5s (human eyes saw)
my patch to kill homescreen in b2g
Ying, this bug is closed, please file a bug to track.
And let facebook issue bug 1007019 depend on your new bug.
Flags: needinfo?(ying.xu)
(In reply to ying.xu from comment #40)
> (In reply to Thinker Li [:sinker] from comment #38)
> > > homescreen in b2g can run faster than OOP, even get killed, also can restore
> > > faster than OOP.
> > > That's the benefit we want to use.
> > 
> > We have better to do measurement of potential side-effects or regressions
                                       ^^^^^^^^^^^^^^^^^^^^^^^^
> the speed of homescreen restoring at this case was about 1s, no more than
> 1.5s (human eyes saw)

You don't really measure potential side-effects.
(In reply to Thinker Li [:sinker] from comment #43)
> > > We have better to do measurement of potential side-effects or regressions
>                                        ^^^^^^^^^^^^^^^^^^^^^^^^
> > the speed of homescreen restoring at this case was about 1s, no more than
> > 1.5s (human eyes saw)
> 
> You don't really measure potential side-effects.

Can you tell us how to do the test?
Flags: needinfo?(ying.xu)
You need to log in before you can comment on or make changes to this bug.