559714 - e10s HTTP: disable cache and any other unused large memory consumers

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Jason Duell]

Now that we're shipping HTTP to chrome we should turn off the disk and memory cache in the child process.  There may be other HTTP data structures we can disable as well.  We need to keep the atom table, and we're keeping the socket transport thread around to service FTP.

Comment 1

[Benjamin Smedberg]

We should fix FTP. I thought we decided that we weren't going to support loading FTP content in-browser, just downloading it, and so the content process didn't need to handle FTP. We shouldn't have:

* any socket transport goop
* NSS/PSM
* caches

We should probably add assertions/aborts to the constructors of those services so that we see them.

Comment 2

[Jason Duell]

I agree about NSS/PSM.  We may need to keep a small cache to support FTP directory listings.

For FTP see bug 536289.  Perhaps we're on the wrong track there, but the plan has been to keep it running in content.

Comment 3

[Johnny Stenback (:jst)]

Over to Michal to work on at least disabling the disk cache in the child process.

Comment 4

[Jason Duell]

As mentioned in the conference call today, we're going to turn off the entire socket transport thread and not have FTP run in content, so this may mean you can turn off more things than previously indicated.  Feel free to open separate bugs as seems sensible.

Comment 5

[Michal Novotny [:michal]]

Attachment: turn off the cache

Comment 6

[Michal Novotny [:michal]]

Attachment: turn off socket transport service

Comment 7

[Michal Novotny [:michal]]

The DNS service is used in the child process by nsHTMLDNSPrefetch but AFAICS this doesn't work correctly because the prefetch should be done in the parent process. So we should call DNS service via IPC and disable it in the child, right?

Comment 8

[Jason Duell]

Comment on attachment 443504 [details] [diff] [review]
turn off the cache

Looks fine to me, but I want biesi to look at it if possible, just to be sure.

Comment 9

[Jason Duell]

Comment on attachment 443506 [details] [diff] [review]
turn off socket transport service

Also looks fine, but I want biesi to look at it if possible, just to be sure.

Comment 10

[Jason Duell]

> The DNS service is used in the child process by nsHTMLDNSPrefetch but AFAICS
this doesn't work correctly because the prefetch should be done in the parent
process. So we should call DNS service via IPC and disable it in the child,
right?

That makes sense to me.  Cc'ing les biez, as they ought to know for sure.

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

Indeed.

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 443506 [details] [diff] [review]
turn off socket transport service

+++ b/netwerk/protocol/http/src/nsHttpHandler.cpp
+#ifdef MOZ_IPC
+    if (!mozilla::net::IsNeckoChild()) {
+#endif
     rv = InitConnectionMgr();
     if (NS_FAILED(rv)) return rv;
+#ifdef MOZ_IPC
+    }
+#endif

I think this one would be better inside of InitConectionMgr, that way it can better ensure that it doesn't get created. Note that there is another call to InitConnectionMgr in this file.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Yeah, dns prefetch needs to move out of the child process.

Comment 14

[Jason Duell]

Note that we don't want to land these patches until we've landed FTP in chrome, etc.

Comment 15

[Michal Novotny [:michal]]

Attachment: turn off socket transport service v2

Comment 16

[Johnny Stenback (:jst)]

The two patches in this bug are ready to land, it seems. Can someone get them in? Is there more to do here, past those two patches?

Comment 17

[Michal Novotny [:michal]]

I'll create a patch to turn off the DNS service too.

The patches can't land until bugs 536289 and 565163 are fixed.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

If the patches can't land yet, please don't mark them checkin-needed.

Comment 19

[Jason Duell]

I'm taking this.  Hope to have done very soon.  Some clients may need changes to behave well in complete absence of nsICacheService.

Comment 20

[Michal Novotny [:michal]]

Bug 536289 is still opened, so these patches can't land.

Comment 21

[Jason Duell]

We should be able to land the patch that disables the cache, which is the big size win, and split disabling the socket transport thread into a different bug.

The existing FTP code handles not having a cache service.  I'm going through the other protocols and uses of the cache service to see if there's anything that needs tweaking to work w/o the cache svc.

Comment 22

[Jason Duell]

Turning off the cache is possibly problematic for wyciwyg channels (in unmodified state--we have patch for e10s version in bug 561085) 

In interim, landed bug 595293, which turns off disk cache in the child, and shrinks memory cache to 1 MB.

This bug should be changed to blocking-fennec-2.0

Comment 23

[[e-mail]]

I'd argue for b2+, since wyciwyg and FTP are marked as such, so we should be able to do this all in one fell swoop. :)

Comment 24

[Jason Duell]

Comment on attachment 443504 [details] [diff] [review]
turn off the cache

Note that we should remove the changes from the patch for bug 595293 when we land this.

Comment 25

[Josh Matthews [:jdm]]

Attachment: Turn off socket transport service in the child process

Unbitrot.

Comment 26

[Josh Matthews [:jdm]]

Attachment: Turn off socket transport service in the child process

Unbitrot.

Comment 27

[Josh Matthews [:jdm]]

Attachment: Turn off cache in the child process

Comment 28

[Josh Matthews [:jdm]]

Turning off the socket transport is a nonstarter, it turns out, since it's needed by nsServerSocket which is created by httpd.js which is launched in child netwerk ipc tests.

Comment 29

[Jason Duell]

Hmm, allegedly child processes can't open sockets in Android (or maybe that's just regular file handles?).  I wonder how httpd.js works in Android if that's true.

Comment 30

[Jason Duell]

Comment on attachment 484448 [details] [diff] [review]
Turn off cache in the child process

Passes try. Let's land and open a followup for turning off the socket transport thread (or not).

Comment 31

[Jason Duell]

Oh, and please back out the patch from 

  http://hg.mozilla.org/mozilla-central/rev/d20225a9200a

as part of landing this.  They won't break anything if they remain, but they're now superfluous with the cache turned off.

Comment 32

[Josh Matthews [:jdm]]

http://hg.mozilla.org/mozilla-central/rev/4ca3a0191455

Comment 33

[Josh Matthews [:jdm]]

Ack, forgot about comment 31.

Comment 34

[Josh Matthews [:jdm]]

Comment 31 addressed by http://hg.mozilla.org/mozilla-central/rev/ddc9d5258911

Comment 35

[Michael Wu [:mwu]]

(In reply to comment #34)
> Comment 31 addressed by http://hg.mozilla.org/mozilla-central/rev/ddc9d5258911

which I just backed out in http://hg.mozilla.org/mozilla-central/rev/10a6c004b114

http://tinderbox.mozilla.org/showlog.cgi?log=Mobile/1287942026.1287942118.30460.gz

Comment 36

[Josh Matthews [:jdm]]

This bug is resolved.  We can still back out the push from bug 595293, but it's not going to cause any problems if we don't.

Comment 37

[Jason Duell]

Mother always told me to compile my patches on at least one platform before checking them in to mozilla-central :)

Re-pushed backout of bug 595293:

  http://hg.mozilla.org/mozilla-central/rev/b67099fcd4d8