807359 - Don't run nsEffectiveTLDService in content processes, or shrink its size

Status: RESOLVED WORKSFORME

(Firefox OS Graveyard :: General, defect)

Description

[Justin Lebar (not reading bugmail)]

Now that we have a memory reporter for nsEffectiveTLDService (bug 802894), we can see whether it makes sense to continue to run this service in content processes.

I would expect it gets called so infrequently that we could proxy calls to it up to the master process no problem and save the ~128kb of memory in each content process.

This probably isn't a high priority, based on the relatively low upper bound on the potential memory win here.

Comment 1

[Nicholas Nethercote [inactive]]

> save the ~128kb of memory in each content process.

Note that in comment bug 802894 comment 2 jduell observed that nsEffectiveTLDService also is responsible for more than 64 KiB of static memory, which the memory reporters don't measure.  So the potential saving is more like 192 KiB.

Comment 2

[Justin Lebar (not reading bugmail)]

> Note that in comment bug 802894 comment 2 jduell observed that nsEffectiveTLDService 
> also is responsible for more than 64 KiB of static memory,

The static data lives in libxul's data segment so is already shared, right?

Comment 3

[Nicholas Nethercote [inactive]]

Oh, quite possibly.

Comment 4

[Justin Lebar (not reading bugmail)]

I see

├──────65,584 B (00.13%) ── xpcom/effective-TLD-service

which is half the size from comment 0.  Looks like comment 0 was parroting a measurement taken on 64-bit (bug 802894 comment 0).

64kb per process is a pretty small potatoes.

Comment 5

[Jason Duell]

To do isThirdParty() checks for cookies requires window objects, which live on the child, so we've moving all those checks child-side (see bug 805616).  And the 3rd party checks require the TLD service.  So we need TLD service on both parent/child.  We could conceivably try to ship the calls to a single TLD svc on the parent, but I think the IPDL delay for a large % of channels, and the code complexity outweigh the 64KB memory savings.

Nice try tho! :)

Comment 6

[Jason Duell]

I'm reopening this, as I suspect that as the TLD database inevitably grows in size (and we seek to find more ways to optimize memory use) we'll want to keep only one copy of it.

From a glance at the code it looks like this would be a matter of 

1) assembling a list of all the URIs from channel/window that we currently synchronously check, and instead shipping them off in a list to the parent to be checked.

2) dealing with making all the code that assumes that IsThirdParty is synchronous. Or, making sure that we're OK with doing a sync IPDL message from the child to do the check.

A fair amount of work for 64KB, but doable at some point.

Comment 7

[Nicholas Nethercote [inactive]]

> I'm reopening this, as I suspect that as the TLD database inevitably grows
> in size (and we seek to find more ways to optimize memory use) we'll want to
> keep only one copy of it.

Is it unchanging data? If so, Nuwa means we should be sharing a single copy of it between all processes. It's not easy to confirm that, however.

Comment 8

[Jason Duell]

Nicholas:

So IIUC Nuwa requires that the TLD data is initialized before we fork child processes.  I just ran gdb until it hit nsEffectiveTLDService::Init (full disclosure: I was lazy and did it in my desktop browser, not a B2G device), and here's the stack trace.  Does nsWindowMemoryReporter::CheckForGhostWindows() run early enough for nuwa, or do we need to stick some earlier call into initialization that grabs the TLD service?

#5  0x00007fffefa67ef4 in CallGetService (aContractID=
    0x7ffff400d5f0 "@mozilla.org/network/effective-tld-service;1", aIID=..., aResult=
    0x7fffffffc248) at /home/jj/hg/c/t/xpcom/glue/nsComponentManagerUtils.cpp:64
#6  0x00007fffefa6842e in nsGetServiceByContractID::operator() (this=0x7fffffffc230, aIID=
    ..., aInstancePtr=0x7fffffffc248)
    at /home/jj/hg/c/t/xpcom/glue/nsComponentManagerUtils.cpp:252
#7  0x00007fffefd74157 in nsCOMPtr<nsIEffectiveTLDService>::assign_from_gs_contractid (this=
    0x7fffffffc300, gs=..., aIID=...) at ../../../dist/include/nsCOMPtr.h:1242
#8  0x00007fffefd73a61 in nsCOMPtr<nsIEffectiveTLDService>::nsCOMPtr (this=0x7fffffffc300, gs=
    ...) at ../../../dist/include/nsCOMPtr.h:623
#9  0x00007ffff1265ccf in nsWindowMemoryReporter::CheckForGhostWindows (this=0x7fffe24346d0, 
    aOutGhostIDs=0x0) at /home/jj/hg/c/t/dom/base/nsWindowMemoryReporter.cpp:798
#10 0x00007ffff126570d in nsWindowMemoryReporter::CheckForGhostWindowsCallback (this=
    0x7fffe24346d0) at /home/jj/hg/c/t/dom/base/nsWindowMemoryReporter.cpp:671
#11 0x00007ffff126b817 in nsRunnableMethodImpl<void (nsWindowMemoryReporter::*)(), void, true>::Run (this=0x7fffd1e4f490) at ../../dist/include/nsThreadUtils.h:383
#12 0x00007fffefb34a55 in nsThread::ProcessNextEvent (this=0x7ffff6a63e20, mayWait=false, 
    result=0x7fffffffc43f) at /home/jj/hg/c/t/xpcom/threads/nsThread.cpp:643
#13 0x00007fffefa758c4 in NS_ProcessNextEvent (thread=0x7ffff6a63e20, mayWait=false)
    at /home/jj/hg/c/t/xpcom/glue/nsThreadUtils.cpp:263
#14 0x00007fffeffaa6f5 in mozilla::ipc::MessagePump::Run (this=0x7fffe2646680, aDelegate=
    0x7fffe2602840) at /home/jj/hg/c/t/ipc/glue/MessagePump.cpp:95
#15 0x00007fffeff4cb13 in MessageLoop::RunInternal (this=0x7fffe2602840)
    at /home/jj/hg/c/t/ipc/chromium/src/base/message_loop.cc:226
---Type <return> to continue, or q <return> to quit---
#16 0x00007fffeff4caa4 in MessageLoop::RunHandler (this=0x7fffe2602840)
    at /home/jj/hg/c/t/ipc/chromium/src/base/message_loop.cc:219
#17 0x00007fffeff4ca35 in MessageLoop::Run (this=0x7fffe2602840)
    at /home/jj/hg/c/t/ipc/chromium/src/base/message_loop.cc:193
#18 0x00007ffff0fb6570 in nsBaseAppShell::Run (this=0x7fffdf2265c0)
    at /home/jj/hg/c/t/widget/xpwidgets/nsBaseAppShell.cpp:164
#19 0x00007ffff265f70c in nsAppStartup::Run (this=0x7fffdf22c3d0)
    at /home/jj/hg/c/t/toolkit/components/startup/nsAppStartup.cpp:276
#20 0x00007ffff254f321 in XREMain::XRE_mainRun (this=0x7fffffffc9a0)
    at /home/jj/hg/c/t/toolkit/xre/nsAppRunner.cpp:3954
#21 0x00007ffff254f63c in XREMain::XRE_main (this=0x7fffffffc9a0, argc=4, argv=
    0x7fffffffdf08, aAppData=0x7fffffffcb50)
    at /home/jj/hg/c/t/toolkit/xre/nsAppRunner.cpp:4021
#22 0x00007ffff254f8b8 in XRE_main (argc=4, argv=0x7fffffffdf08, aAppData=0x7fffffffcb50, 
    aFlags=0) at /home/jj/hg/c/t/toolkit/xre/nsAppRunner.cpp:4231
#23 0x00000000004046a0 in do_main (argc=4, argv=0x7fffffffdf08, xreDirectory=0x7ffff6a2b480)
    at /home/jj/hg/c/t/browser/app/nsBrowserApp.cpp:282
#24 0x0000000000404aed in main (argc=4, argv=0x7fffffffdf08)
    at /home/jj/hg/c/t/browser/app/nsBrowserApp.cpp:643

Comment 9

[Nicholas Nethercote [inactive]]

Cervantes or Thinker should be able to answer the question in comment 8 better than I can.

Comment 10

[Thinker Li [:sinker]]

For now, Nuwa is frozen after RecvAppInfo(), from IPC, where PreloadSlowThing() is called. It is started by post a runnable. So, all runnables posted before nuwa's one is expected to be ran before being frozen.  XPCOM is initialized before posting nuwa's runnable, so you have to call nsEffectiveTLDService::Init() during initialization of XPCOM, or post a runnable before nuwa's runnable being ran.

CheckForGhostWindows() seems too late; triggered by the destroy of some window.  How about toinitialize nsEffeictiveTLDService during PreloadSlowThing()? (see dom/ipc/preload.js)

Comment 11

[Nicholas Nethercote [inactive]]

Is CheckForGhostWindows() really the first place that initializes nsEffectiveTLDService? That's odd -- that function only runs when (a) a DOM window is destroyed (i.e. an DOM_WINDOW_DESTROYED_TOPIC observer event occurs) or (b) the memory reporters are run, e.g. by being triggered from about:memory.

As Thinker says, initializing it eagerly might be the solution.

Comment 12

[Cervantes Yu [:cyu] [:cervantes]]

I checked this on the device and it's initialized early in the Nuwa process:

#44 nsAppStartupNotifier::Observe (this=<value optimized out>, aSubject=<value optimized out>, aTopic=0x41903952 "app-startup", someData=<value optimized out>)
#45 0x4127ec80 in XRE_InitEmbedding2 (aLibXULDirectory=<value optimized out>, aAppDirectory=<value optimized out>, aAppDirProvider=0x0)
#46 0x408182f0 in mozilla::ipc::ScopedXREEmbed::Start (this=0x4033ee48)
#47 0x40c38e4e in mozilla::dom::ContentProcess::Init (this=0x4033ec00)
#48 0x4127ea9e in XRE_InitChildProcess (aArgc=-1098831656, aArgv=0xbe81284c, aProcess=1077144576)

calls into JS:
p xpc_PrintJSStack(0x42f29740,0,0,0)
$1 = 0x40311ce0 "0 anonymous() [\"resource://gre/modules/CSPUtils.jsm\":37]\n1 anonymous() [\"jar:file:///system/b2g/omni.ja!/components/ProcessGlobal.js\":26]\n"

and then nsEffectiveTLDService::Init() is called. So it's initialized once in the Nuwa process.

Comment 13

[Jason Duell]

Somebody already thought of this for B2G and stuck an init line for the TLD service in preload.js:

  http://mxr.mozilla.org/mozilla-central/source/dom/ipc/preload.js#54

So we're already sharing this memory--yay!

Thanks for the followup everyone!