Closed Bug 1014272 Opened 11 years ago Closed 11 years ago

[tarako only] Run the homescreen in process

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia::System
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

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

Status:
RESOLVED FIXED
Project Flags:
blocking-b2g 1.3T+
Tracking Flags:
Tracking Status
b2g-v1.3T --- fixed

People

(Reporter: fabrice, Assigned: fabrice)

References

Details

(Whiteboard: [sprd295070][tarako_only])

Attachments

(2 files, 1 obsolete file)

homescreen-inproc.patch
1.14 KB, patch
alive
: review+
Details | Diff | Splinter Review
kill-homescreen.patch
560 bytes, patch
Details | Diff | Splinter Review
Yes, it's sad. But that gives us a better user experience overall from my testing.
Attached 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.
status-b2g-v2.0: fixed → ---
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
status-b2g-v2.0: --- → fixed
Whiteboard: [sprd295070][tarako_only] → [sprd295070]
status-b2g-v2.0: fixed → ---
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.

Attachment

General

Creator:
Created:
Updated:
Size: