Closed Bug 992752 Opened 6 years ago Closed 6 years ago

[tarako] In consecutive app launches, second one needs to wait until the delayed preallocated process

Categories

(Firefox OS Graveyard :: General, defect, P1, major)

ARM
Gonk (Firefox OS)

Tracking

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

RESOLVED FIXED
1.4 S6 (25apr)
blocking-b2g 1.3T+
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)

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
    }
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)
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
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
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)
The above push log doesn't include unit tests. Here is the new one: https://tbpl.mozilla.org/?tree=Try&rev=f85a8b23c96e
Whiteboard: [c=progress p= s= u=tarako]
Renom for 1.3T for launch performance in quick app launches.
blocking-b2g: --- → 1.3T?
triage: this patch seems relatively simple, we take this to improve tarako app launch performance
blocking-b2g: 1.3T? → 1.3T+
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 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?
Flags: needinfo?(fabrice)
Whiteboard: [c=progress p= s= u=tarako] → [c=progress p= s= u=tarako], [tarako_only]
I'll uplift once it's on m-c
Flags: needinfo?(fabrice)
https://hg.mozilla.org/mozilla-central/rev/d60ec8441e4c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S1 (9may)
Priority: -- → P1
Whiteboard: [c=progress p= s= u=tarako], [tarako_only] → [c=progress p= s=2014.04.25 u=tarako], [tarako_only]
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.