Closed Bug 930282 Opened 11 years ago Closed 10 years ago

Enable the Nuwa process on B2G by default

Categories

(Core :: IPC, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla29
blocking-b2g 1.3+
Tracking Status
firefox27 --- wontfix
firefox28 --- fixed
firefox29 --- fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed

People

(Reporter: cyu, Assigned: kk1fff)

References

Details

(Whiteboard: [MemShrink:P1])

Attachments

(3 files, 2 obsolete files)

We landed the Nuwa process enhancement in bug 771765, but it is not turned on by default. This is to track the enablement of the Nuwa process.
blocking-b2g: --- → 1.3?
(1.3+'ing because we aren't going to carry bug 811671 forward on jb-gonk so we need this bug instead to avoid USS regression)
blocking-b2g: 1.3? → 1.3+
Do we need to do only |export MOZ_NUWA_PROCESS=1| to enable NUWA patch https://bugzilla.mozilla.org/show_bug.cgi?id=771765#c213 .
Flags: needinfo?(cyu)
(In reply to Tapas Kumar Kundu from comment #2)
> Do we need to do only |export MOZ_NUWA_PROCESS=1| to enable NUWA patch
Currently yes. It'll be enabled by default after current pending issues are resolved.
Flags: needinfo?(cyu)
(In reply to Cervantes Yu from comment #3)
> (In reply to Tapas Kumar Kundu from comment #2)
> > Do we need to do only |export MOZ_NUWA_PROCESS=1| to enable NUWA patch
> Currently yes. It'll be enabled by default after current pending issues are
> resolved.

Could you please tell me what are those pending issues ? If I enable it using MOZ_NUWA_PROCESS=1 then it won't be effected much till we see solution from those pending issues . Correct ? 

I am trying to evaluate gains from NUWA patch :)
Flags: needinfo?(cyu)
I too am trying to evaluate gains from the NUWA implementation.  On an inari I see regression.  Cross compiled and flashed from today's m-c, with and without MOZ_NUWA_PROCESS defined.  Full rebuild from each with distinct obj dir.  I see the Nuwa template process spawned as expected, but no appreaciable reduction in USS.  Am I doing it wrong?

before: https://gist.github.com/lloyd/7371318
after: https://gist.github.com/lloyd/7371306

(also note, teensy tinsy bug in b2g-info's parsing of proc title, doesn't handle embedded parens)
Whiteboard: [MemShrink]
Whiteboard: [MemShrink] → [MemShrink:P1]
Depends on: 922465
Flags: needinfo?(cyu)
Depends on: 937018
(In reply to Tapas Kumar Kundu from comment #4)
> (In reply to Cervantes Yu from comment #3)
> > (In reply to Tapas Kumar Kundu from comment #2)
> > > Do we need to do only |export MOZ_NUWA_PROCESS=1| to enable NUWA patch
> > Currently yes. It'll be enabled by default after current pending issues are
> > resolved.
> 
> Could you please tell me what are those pending issues ? If I enable it
> using MOZ_NUWA_PROCESS=1 then it won't be effected much till we see solution
> from those pending issues . Correct ? 
> 
The main pending issue is the automated test for the Nuwa process (bug 922465). Once it is landed, we can turn on the option on Gonk by default so if there are some change that regress the Nuwa process (like some new thread created during startup, e.g. bug 937018), we can detect it using TPBL.
> I am trying to evaluate gains from NUWA patch :)
(In reply to Lloyd Hilaiel [:lloyd] from comment #5)
> I too am trying to evaluate gains from the NUWA implementation.  On an inari
> I see regression.  Cross compiled and flashed from today's m-c, with and
> without MOZ_NUWA_PROCESS defined.  Full rebuild from each with distinct obj
> dir.  I see the Nuwa template process spawned as expected, but no
> appreaciable reduction in USS.  Am I doing it wrong?
> 
> before: https://gist.github.com/lloyd/7371318
> after: https://gist.github.com/lloyd/7371306
> 
From your testing result, I think you encountered bug 937018 and your Nuwa process is not fully functional. So the Nuwa process is just yet another process that consumes some memory but is not forking content processes.
To verify that the Nuwa process is working, use b2g-ps to show relationships between processes like:

APPLICATION      USER     PID   PPID  VSIZE  RSS     WCHAN    PC         NAME
b2g              root      12175 1     176480 68892 ffffffff 400b24e0 S /system/b2g/b2g
(Nuwa)           root      12203 12175 52852  23636 ffffffff 401054e0 S /system/b2g/plugin-container
Usage            app_12266 12266 12175 69716  29124 ffffffff 400cd4e0 S /system/b2g/plugin-container
Homescreen       app_12300 12300 12175 79240  33928 ffffffff 400904e0 S /system/b2g/plugin-container
(Preallocated a  root      12448 12203 56940  14384 ffffffff 401054e0 S /system/b2g/plugin-container

Note that parent of process 12448 is 12203, not the b2g process. If you see this, then the Nuwa process is functional.

I perform a similar test w/wo the Nuwa process, here are my results:

Without the Nuwa process:

root@android:/ # b2g-info
                      |     megabytes    |
      NAME   PID NICE  USS  PSS  RSS VSIZE OOM_ADJ USER
       b2g 12900    0 48.2 50.8 63.3 166.2       0 root
     Usage 12959   18 12.5 14.7 26.5  66.0      12 app_12959
Homescreen 12984   18 15.3 17.7 30.0  70.7       8 app_12984
    E-Mail 13073   18 17.2 19.6 31.7  74.8      11 app_13073
  Calendar 13090   18 13.0 15.4 27.5  67.7      11 app_13090
  Settings 13108   18 13.5 16.0 28.3  68.8      10 app_13108

System memory info:

            Total 183.8 MB
     Used - cache 137.3 MB
  B2G procs (PSS) 134.3 MB
    Non-B2G procs   3.1 MB
     Free + cache  46.4 MB
             Free  10.1 MB
            Cache  36.3 MB

With the Nuwa process (bug 937019's patch is applied). You can see the reduction of USS and PSS in Email, Calendar, and Settings.

                           |     megabytes    |
           NAME   PID NICE  USS  PSS  RSS VSIZE OOM_ADJ USER
            b2g 12175    0 51.0 53.7 66.2 169.3       0 root
                12203    0  4.3  7.8 22.6  51.6       2 root
          Usage 12266   18 12.7 15.0 26.7  66.0      12 app_12266
     Homescreen 12300    1 16.1 18.6 30.9  75.8       2 app_12300
         E-Mail 12448   18 12.1 14.6 28.1  66.8      11 app_12448
       Calendar 12586   18  8.0 10.5 24.0  60.7      11 app_12586
       Settings 12612   18  9.1 11.8 25.5  64.4      10 app_12612
(Preallocated a 12633   18  2.9  5.0 14.1  55.6      10 root

System memory info:

            Total 183.8 MB
     Used - cache 140.5 MB
  B2G procs (PSS) 137.2 MB
    Non-B2G procs   3.4 MB
     Free + cache  43.2 MB
             Free   4.4 MB
            Cache  38.9 MB
Thank you for the thorough and educational response, cervantes.  Indeed:

root@android:/ # b2g-ps | busybox grep b2g
b2g              root      1646  1     176852 61696 ffffffff 401284e0 S /system/b2g/b2g
(Nuwa)           root      1682  1646  52852  17864 ffffffff 400fe6ec S /system/b2g/plugin-container
Homescreen       app_1806  1806  1646  73448  25212 ffffffff 400664e0 S /system/b2g/plugin-container
Firefox Account  app_2823  2823  1646  75928  23700 ffffffff 400ae4e0 S /system/b2g/plugin-container
Calendar         app_3574  3574  1646  73900  27716 ffffffff 4008c4e0 S /system/b2g/plugin-container
Communications   app_4931  4931  1646  87544  36948 ffffffff 4014a4e0 S /system/b2g/plugin-container

Nuwa is *not* the parent of spawned processes.  I will apply the patch from bug #937018 and re-test.
Holy crapsticks guys, this is fantastic!  After application of that patch (and fixing parsing of b2g-info) I see this:




$ adb shell b2g-info
                          |     megabytes    |
           NAME  PID NICE  USS  PSS  RSS VSIZE OOM_ADJ USER    
            b2g  112    0 53.2 55.0 61.9 166.5       0 root    
         (Nuwa)  246    1  4.3  6.1 13.4  51.6       2 root    
     Homescreen  910   18  9.9 12.3 23.3  62.7       8 app_910 
Firefox Account 1158   18  6.3  9.2 21.2  66.1      11 app_1158
         E-Mail 1212   18 13.6 17.9 31.7  77.2      11 app_1212
       Calendar 1274   18 11.2 15.5 29.4  74.6      10 app_1274
(Preallocated a 1308   18  2.9  4.7 11.9  55.6      10 root    

System memory info:

            Total 172.6 MB
     Used - cache 132.7 MB
  B2G procs (PSS) 120.7 MB
    Non-B2G procs  12.0 MB
     Free + cache  39.9 MB
             Free  18.6 MB
            Cache  21.3 MB

Low-memory killer parameters:

  notify_trigger 14336 KB

  oom_adj min_free
        0  4096 KB
        1  5120 KB
        2  6144 KB
        6  7168 KB
        8  8192 KB
       10 20480 KB

Confirmed proper parent is Nuwa:

$ adb shell b2g-ps
APPLICATION      USER     PID   PPID  VSIZE  RSS     WCHAN    PC         NAME
b2g              root      112   1     167244 62012 ffffffff 400cd4e0 S /system/b2g/b2g
(Nuwa)           root      246   112   52848  13692 ffffffff 401074e0 S /system/b2g/plugin-container
Homescreen       app_910   910   246   64232  23896 ffffffff 401074e0 S /system/b2g/plugin-container
Firefox Account  app_1158  1158  246   67732  21676 ffffffff 401074e0 S /system/b2g/plugin-container
E-Mail           app_1212  1212  246   79052  32436 ffffffff 401074e0 S /system/b2g/plugin-container
Calendar         app_1274  1274  246   75316  30056 ffffffff 401074e0 S /system/b2g/plugin-container
(Preallocated a  root      1308  246   56936  12224 ffffffff 401074e0 S /system/b2g/plugin-container

Some observations:

1. process spinup for a very small process after reboot (empty file cache) used to tak ~1900ms.  Now takes ~630ms. (marketing: processes launch 3x faster)
2. same process went from consuming 9.3 megs of unique memory to 6.3.  (marketing: processes consume 33% less memory)

The one question I have is on initial startup.  From reading code and observation it seems like we do not block creation of initial processes while Nuwa initializes.  Killing HomeScreen and Usage causes them to re-spawn on demand, but just faster spawning and they consume less USS.

Is it worth opening a bug to explore delaying launch of Homescreen and Usage until Nuwa is initialized?

Fantastic!
note - low severity, low priority, purely cosmetic, but nice to have when nuwa lands: https://github.com/mozilla-b2g/gonk-misc/pull/130
(In reply to Lloyd Hilaiel [:lloyd] from comment #9)
> Is it worth opening a bug to explore delaying launch of Homescreen and Usage
> until Nuwa is initialized?
Sure. This is worth investigation.
Blocks: 938470
Yes, if we can save 6MB of memory, that could be worth delaying startup a bit. Please do file a new bug (and cc me).

I'd also love to see if there's anything we can do to further reduce the time needed, and the memory used, when starting Nuwa processes.
(In reply to Jonas Sicking (:sicking) Please ni? if you want feedback from comment #12)
> Yes, if we can save 6MB of memory, that could be worth delaying startup a
> bit. Please do file a new bug (and cc me).
> 
Just add a record here. Looks like bug 938470 is the bug for this. :-)
(In reply to Kevin Hu [:khu] from comment #13)
> (In reply to Jonas Sicking (:sicking) Please ni? if you want feedback from
> comment #12)
> > Yes, if we can save 6MB of memory, that could be worth delaying startup a
> > bit. Please do file a new bug (and cc me).
> > 
> Just add a record here. Looks like bug 938470 is the bug for this. :-)

Thanks for the update Kevin.Can you please help with an owner on 938479, given its assigned to nobody
Flags: needinfo?(khu)
(In reply to bhavana bajaj [:bajaj] from comment #14)
> (In reply to Kevin Hu [:khu] from comment #13)
> > (In reply to Jonas Sicking (:sicking) Please ni? if you want feedback from
> > comment #12)
> > > Yes, if we can save 6MB of memory, that could be worth delaying startup a
> > > bit. Please do file a new bug (and cc me).
> > > 
> > Just add a record here. Looks like bug 938470 is the bug for this. :-)
> 
> Thanks for the update Kevin.Can you please help with an owner on 938479,
> given its assigned to nobody

I think you mean bug 938470. Already ping cyu to see if he could be the owner.
Flags: needinfo?(khu)
Cervantes,

Can you please help understand where we are with the fix for the bug?
Flags: needinfo?(cyu)
Cervantes, can we turn it on asap so we can see what it breaks?
Needed for 1.3 based on comment 1 from Qualcomm.
I enabled it on pine but it looks pretty orange:
https://tbpl.mozilla.org/?tree=Pine&showall=1&rev=d7f9fdb4177b
The status of this bug:

The blocker of this bug is bug 922465, which adds auto tests so regressions to the Nuwa process can be detected in buildbot automation.

We have a working WIP for it, but it disables PreloadSlowThings() in the Nuwa process to make tests on emulators green. We figured out that emulators runs too slow compared to real devices so the Nuwa process blocks on async events generated in the preloaded JS in PreloadSlowThings().

The potential bug that makes the Nuwa process freeze is tracked in 941466 and we target on next Monday for a patch for review.
Flags: needinfo?(cyu)
(In reply to Cervantes Yu from comment #20)
> The status of this bug:
> 
> The blocker of this bug is bug 922465, which adds auto tests so regressions
> to the Nuwa process can be detected in buildbot automation.
> 
> We have a working WIP for it, but it disables PreloadSlowThings() in the
> Nuwa process to make tests on emulators green. We figured out that emulators
> runs too slow compared to real devices so the Nuwa process blocks on async
> events generated in the preloaded JS in PreloadSlowThings().

Hm I don't see this. The tests pass on debug-emulator and fail on opt-emulator. This seems to be a race and not a performance problem.
Depends on: 944665
Attached patch Part 1: Enable Nuwa. (obsolete) — Splinter Review
Attachment #8341015 - Attachment description: Part 1: Mark PACMan thread and BackgroundHangMonitor. → Part 2: Mark PACMan thread and BackgroundHangMonitor.
Comment on attachment 8341016 [details] [diff] [review]
Part 1: Enable Nuwa.

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

Also change the environment variable that is used to enable nuwa in b2g/confvar.sh. Since the name is the same as the environment variable we use in make files, it would make Nuwa.cpp be compiled in b2g desktop build.
Attachment #8341016 - Flags: feedback?(cyu)
Attachment #8341015 - Flags: review?(khuey)
Comment on attachment 8341016 [details] [diff] [review]
Part 1: Enable Nuwa.

I think we don't need to introduce yet another variable: we can add checks in confvars.sh and prevent it from building in b2g-desktop. Using only one variable looks simpler.
Attachment #8341016 - Flags: feedback?(cyu)
Address Cervantes' comment. Much cleaner now. Thanks!
Attachment #8341016 - Attachment is obsolete: true
Attachment #8341606 - Flags: review?(khuey)
Comment on attachment 8341606 [details] [diff] [review]
Part 1: Enable Nuwa.

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

\o/
Attachment #8341606 - Flags: review?(khuey) → review+
Patrick, does it even make sense to have PAC running on b2g?
Flags: needinfo?(mcmanus)
pac can be used in funny ways - I'm really not sure if b2g allows any of them. e.g. a pac file is used to control various routing bits in the mochitests. Carriers are also going to want proxy configs and PAC is a reasonable way of doing some kind of a "am I on the telco network or not" test to see if it should be used for any particular transaction.. so I would think we would want it.
Flags: needinfo?(mcmanus)
Need-info Fabrice (1.3 sheriff) to get approval for landing this for 1.3 when review of test case (bug 922465) completed.
Flags: needinfo?(fabrice)
(In reply to Patrick Wang [:kk1fff] from comment #31)
> Need-info Fabrice (1.3 sheriff) to get approval for landing this for 1.3
> when review of test case (bug 922465) completed.

Feel free to land once the tests are ready.
Flags: needinfo?(fabrice)
Please make sure you test Nuwa on a debug build of b2g before landing.
Assignee: nobody → pwang
https://hg.mozilla.org/mozilla-central/rev/625f5303fc68
https://hg.mozilla.org/mozilla-central/rev/c4f970dd2a4f
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Backed out in https://hg.mozilla.org/mozilla-central/rev/06b3a7aea2c0 for frequent (somewhere between 20 and 50%) xpcshell shutdown crashes like https://tbpl.mozilla.org/php/getParsedLog.php?id=31651189&tree=B2g-Inbound
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla28 → ---
Whiteboard: [MemShrink:P1] → [MemShrink:P1][tarako]
(In reply to Phil Ringnalda (:philor) from comment #36)
> Backed out in https://hg.mozilla.org/mozilla-central/rev/06b3a7aea2c0 for
> frequent (somewhere between 20 and 50%) xpcshell shutdown crashes like
> https://tbpl.mozilla.org/php/getParsedLog.php?id=31651189&tree=B2g-Inbound

Looks like we schedule nuwa fork before test start, and nuwa forked after xpcom shutdown. I think shouldn't fork nuwa after xpcom shutdown.
Attachment #8344659 - Flags: review?(khuey)
Comment on attachment 8344659 [details] [diff] [review]
Patch: Part 3: Don't fork after xpcom shutdown.

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

I don't think we should be starting Nuwa in xpcshell at all, but we can deal with that in another bug.

r=me
Attachment #8344659 - Flags: review?(khuey) → review+
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #39)
> I don't think we should be starting Nuwa in xpcshell at all, but we can deal
> with that in another bug.

I will file a follow-up for this.
Blocks: 948021
Hey guys, I would have appreciated some memory usage numbers before we enabled that by default. From what I see on 1.3, since we still have the pre-allocated process, we are actually using more memory with nuwa processes on overall. So I'd like nuwa to be disabled until we have data on memory usage.

Is there a bug to get rid of the pre-allocated process and to use only the nuwa process?
And I'd like to understand the difference between the pre-allocated process and the Nuwa process.  My understanding at the moment is that they do much the same thing -- provide a basic process template which can be forked to create new processes -- but that Nuwa has a lot more stuff initialized, and so facilitates more sharing.  Is that right?
https://hg.mozilla.org/mozilla-central/rev/19124ad87c71
https://hg.mozilla.org/mozilla-central/rev/3fa57d5d6db0
https://hg.mozilla.org/mozilla-central/rev/ecfccd18bcf2
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
(In reply to Nicholas Nethercote [:njn] from comment #43)
> And I'd like to understand the difference between the pre-allocated process
> and the Nuwa process.  My understanding at the moment is that they do much
> the same thing -- provide a basic process template which can be forked to
> create new processes -- but that Nuwa has a lot more stuff initialized, and
> so facilitates more sharing.  Is that right?

Not quite, at least not as I understand it: The preallocated process is transformed into a single content process (it's the regular fork + exec(plugin-container) + become-content-process procedure, with the fork/exec done ahead of time) but the Nuwa process forks and its children (the main process's grandchildren) become content processes.
Or, put another way: The pre-allocated process is an optimization of child process startup latency, by doing most of the init ahead of time.  Nuwa is a memory optimization (trading a constant overhead — the USS of the Nuwa process — for the per-child gain of everything that's not COW-broken), which improves non-preallocated child startup latency as a side effect and probably not quite as much as preallocation does.
After the enablement of the Nuwa process, we still preallocate a process for apps. The difference is that the preallocated process is b2g's grandchild (and preallocating this process has a lower cost).

If we need a remote iframe when the Nuwa-preallocated process is not ready, we could still fall back to the current fork() + exec() content process. This happens to the homescreen and cost control apps during startup, and maybe consecutive launching requests (but this is unlikely for normal usage). Bug 938470 aims to fix this.
It's still unclear (and yes, I want data) if using the nuwa process *and* the preallocated one is a win overall. Can we get numbers with the same set of apps running, before and after we enable that?
A preallocated process cost about ~10MB (unique pages).  For every nuwa process could reduce ~4MB of memory.  It means that Nuwa wins it for 3+ apps from Nuwa for the case of nuwa *and* the preallocated one.
Yeah, so when using just system app + homescreen and a browser tab we're in a worse place with nuwa...

Thinker, do you think we could get rid of the preallocated process?
After bug 938470, we could get rid of the preallocated process.  Overall, Nuwa does not only improve memory usage, also improve launch time by avoidng I/O requests of creating new processes. The time span of creating the preallocated process would be overlaid with the launch time of the foreground app.  It can slow down the launch time of the app, or slow down the foreground app for 1 or more seconds.  However, I will find some one to study the launch time of bug 938470.
This needs to be backed out on all branches. This is causing the following problems:

* The about memory script fails on b2g right now (bug 948774)
* Random crashes during basic usage of the phone (bug 948485)
* A perma orange in the tree (bug 948545)

Ryan - Can you back this out on all branches?
Flags: needinfo?(ryanvm)
The correct way to disable this is just to flip the variable in confvars.sh, not to back it out.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #54)
> The correct way to disable this is just to flip the variable in confvars.sh,
> not to back it out.

Okay - makes sense.

Patrick - Can you disable this until we resolve the about:memory issue?
Flags: needinfo?(ryanvm) → needinfo?(pwang)
sure, I can make a patch.
Flags: needinfo?(pwang)
Depends on: 948829
So this bug is resolved fixed, but bug 948829 disabled Nuwa.  AFAICT it's still disabled.  Do we have another bug for re-enabling Nuwa?
I don't think so.  Want to file one?
 status-b2g-v1.3T fixed, remove [tarako]
Whiteboard: [MemShrink:P1][tarako] → [MemShrink:P1]
Depends on: 974807
No longer depends on: 974807
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: