Closed Bug 930282 Opened 11 years ago Closed 11 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) (deleted) — 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)
Attached patch Part 1: Enable Nuwa. (deleted) — Splinter Review
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
Status: NEW → RESOLVED
Closed: 11 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?
Status: REOPENED → RESOLVED
Closed: 11 years ago11 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: