Don't create a new preallocated process so soon after a process is started

RESOLVED FIXED
(NeedInfo fromanyone)

Status

Firefox OS
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Justin Lebar (not reading bugmail), Assigned: Justin Lebar (not reading bugmail), NeedInfo)

Tracking

unspecified

Firefox Tracking Flags

(blocking-b2g:tef+, firefox21 wontfix, firefox22 wontfix, firefox23 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
I don't know if I regressed this or we were always so bad, but I observe that right after we receive a call, almost the first thing we do is launch a new preallocated process!

That's quite bad; we're just wasting CPU time.

This is easy enough to fix, I think.
(Assignee)

Comment 1

5 years ago
Created attachment 747223 [details] [diff] [review]
Patch, v1
Attachment #747223 - Flags: review?(bent.mozilla)
(Assignee)

Updated

5 years ago
Assignee: nobody → justin.lebar+bug
(Assignee)

Updated

5 years ago
blocking-b2g: --- → tef+
Whiteboard: [status: awaiting review]
Hmm. So when I originally coded this, the first idle didn't occur until several seconds after launching a new app.

Maybe the dialer is different, in that it actually blocks and waits for something right away.

What we discovered with other apps, was that the timeout (which was 5 seconds I think) was occuring before the app had reached idle, so the preallocated process was slowing down the app initialization.

Perhaps we need something like first idle after some delay.
(Assignee)

Comment 3

5 years ago
> Maybe the dialer is different, in that it actually blocks and waits for something right away.

Or perhaps idle happens sooner when we're under CPU stress.

> Perhaps we need something like first idle after some delay.

This is basically delay-after-first-idle instead of first-idle-after-delay.  I hope it's close enough; it's certainly much easier to do.
(In reply to Justin Lebar [:jlebar] from comment #3)
> > Maybe the dialer is different, in that it actually blocks and waits for something right away.
> 
> Or perhaps idle happens sooner when we're under CPU stress.

IIRC The idle is that the child is idle, which means that it has no messages to process. This typically would happen when it's waiting for async I/O to complete (or the user to press a mouse or something).

Maybe the idle stuff has regressed from moving stuff off the main thread to other threads, and the idle isn't "good enough" any more.
(Assignee)

Comment 5

5 years ago
> IIRC The idle is that the child is idle, which means that it has no messages to process. 

Right, but the child could become idle if, for example, it's not being pumped as quickly with messages from the main process.  Which could happen under certain types of stress.
(Assignee)

Updated

5 years ago
Attachment #747223 - Flags: review?(bent.mozilla) → review?(dhylands)
Comment on attachment 747223 [details] [diff] [review]
Patch, v1

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

This seems like a reasonable compromise for the time being.
Attachment #747223 - Flags: review?(dhylands) → review+
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/projects/birch/rev/74dbf4035f07

Updated

5 years ago
Whiteboard: [status: awaiting review] → [status: needs uplift]
https://hg.mozilla.org/mozilla-central/rev/74dbf4035f07
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
https://hg.mozilla.org/releases/mozilla-b2g18/rev/d1b25094d1fd
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/072e642ad2b7
status-b2g18: --- → fixed
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → fixed
status-firefox21: --- → wontfix
status-firefox22: --- → wontfix
status-firefox23: --- → fixed
Whiteboard: [status: needs uplift]
Can you please provide steps to verify this fix - as we will blackbox test from the UI?

Comment 11

5 years ago
Can you please provide steps to verify this fix - as we will blackbox test from the UI?
Flags: needinfo?
You need to log in before you can comment on or make changes to this bug.