Closed
Bug 992752
Opened 10 years ago
Closed 10 years ago
[tarako] In consecutive app launches, second one needs to wait until the delayed preallocated process
Categories
(Firefox OS Graveyard :: General, defect, P1)
Tracking
(blocking-b2g:1.3T+, b2g-v1.3T fixed)
Tracking | Status | |
---|---|---|
b2g-v1.3T | --- | fixed |
People
(Reporter: ying.xu, Assigned: cyu)
Details
(Keywords: perf, Whiteboard: [c=progress p= s=2014.04.25 u=tarako], [tarako_only])
Attachments
(1 file)
1.07 KB,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
which means we have wait at least processPrelaunch.delayMs ms(5000ms now) to run another app after launching one.
code as below: Can we add a new sync-interface to create the preallocated process? wait here is not good // Failed in using the preallocated process: fork from the chrome process. if (!p) { #ifdef MOZ_NUWA_PROCESS if (Preferences::GetBool("dom.ipc.processPrelaunch.enabled", false)) { // Wait until the Nuwa process forks a new process. return nullptr; } #else p = new ContentParent(/* app = */ nullptr, aForBrowserElement, /* isForPreallocated = */ false, base::PRIVILEGES_DEFAULT, PROCESS_PRIORITY_FOREGROUND); #endif }
Assignee | ||
Comment 2•10 years ago
|
||
Please note that forking a new content process needs IPC with Nuwa, which is async in nature. The code here doesn't mean that we need to wait for the delayed fork of the preallocated process. There is urgent fork request like that we need to launch a new app, but there is no preallocated process. In this case we will fork immediately. Do you see real bug that the system is idle and we launch a new app after delay?
Flags: needinfo?(ying.xu)
Here is my test. Change delayMS to a big value, such as 50000. entering homescreen and launch an app then launch another The first one can be started because there is a prelaunch process. But The second app would not be started until wait for 50000 ms. diff --git a/b2g/app/b2g.js b/b2g/app/b2g.js index 39452fb..50e6242 100644 --- a/b2g/app/b2g.js +++ b/b2g/app/b2g.js @@ -695,7 +695,7 @@ pref("gonk.systemMemoryPressureRecoveryPollMS", 5000); // (hiding latency). pref("dom.ipc.processPrelaunch.enabled", true); // Wait this long before pre-launching a new subprocess. -pref("dom.ipc.processPrelaunch.delayMs", 5000); +pref("dom.ipc.processPrelaunch.delayMs", 50000); #endif pref("dom.ipc.reuse_parent_app", true);
Flags: needinfo?(ying.xu)
Assignee | ||
Comment 4•10 years ago
|
||
OK, the problem is not in CreateBrowserOrApp(). The problem is in PreallocatedProcessManagerImpl::RunAfterPreallocatedProcessReady(). Calling this method always means an urgent request, we should not check the presence of delayed fork request.
Assignee: nobody → cyu
Assignee | ||
Updated•10 years ago
|
Summary: [tarako] CreateBrowserOrApp/GetNewOrUsed need to wait dom.ipc.processPrelaunch.delayMs ms to create a new ContentParent when MOZ_NUWA_PROCESS was enables → [tarako] In consecutive app launches, second one needs to wait until the delayed preallocated process
Assignee | ||
Comment 5•10 years ago
|
||
This fixes the bug that when we launch an app while there is a delayed prealloc task, the launch is delayed until the prealloc task gets executed.
Attachment #8402576 -
Flags: review?(khuey)
Assignee | ||
Comment 6•10 years ago
|
||
Try push log: https://tbpl.mozilla.org/?tree=Try&rev=52061e610fdb
Updated•10 years ago
|
status-b2g-v1.3T:
--- → affected
Keywords: perf
Assignee | ||
Comment 7•10 years ago
|
||
The above push log doesn't include unit tests. Here is the new one: https://tbpl.mozilla.org/?tree=Try&rev=f85a8b23c96e
Attachment #8402576 -
Flags: review?(khuey) → review+
Updated•10 years ago
|
Whiteboard: [c=progress p= s= u=tarako]
Assignee | ||
Comment 8•10 years ago
|
||
Respin linux debug bc tests: https://tbpl.mozilla.org/?tree=Try&rev=1cc806c18c5a
Assignee | ||
Comment 9•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d60ec8441e4c
Assignee | ||
Comment 10•10 years ago
|
||
Renom for 1.3T for launch performance in quick app launches.
blocking-b2g: --- → 1.3T?
Comment 11•10 years ago
|
||
triage: this patch seems relatively simple, we take this to improve tarako app launch performance
blocking-b2g: 1.3T? → 1.3T+
Assignee | ||
Comment 12•10 years ago
|
||
Comment on attachment 8402576 [details] [diff] [review] Don't wait for the delayed prellocated process in when launching an app. [Approval Request Comment] Bug caused by (feature/regressing bug #): 938470 User impact if declined: Longer app launch time if the user launches multiple apps consecutively. Testing completed: manual and try on m-c Risk to taking this patch (and alternatives if risky): Low. The worst case to be multiple preallocated app processes if anything goes wrong.
Attachment #8402576 -
Flags: approval-mozilla-b2g28?
Comment 13•10 years ago
|
||
Comment on attachment 8402576 [details] [diff] [review] Don't wait for the delayed prellocated process in when launching an app. Given this needs to land on 1.3T alone, I am NI :fabrice here to help with uplift
Attachment #8402576 -
Flags: approval-mozilla-b2g28?
Updated•10 years ago
|
Flags: needinfo?(fabrice)
Whiteboard: [c=progress p= s= u=tarako] → [c=progress p= s= u=tarako], [tarako_only]
Comment 15•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d60ec8441e4c
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S1 (9may)
Updated•10 years ago
|
Priority: -- → P1
Whiteboard: [c=progress p= s= u=tarako], [tarako_only] → [c=progress p= s=2014.04.25 u=tarako], [tarako_only]
Updated•10 years ago
|
Target Milestone: 2.0 S1 (9may) → 1.4 S6 (25apr)
You need to log in
before you can comment on or make changes to this bug.
Description
•