1362498 - cache some more xpcom services in nshttphandler

Status: RESOLVED FIXED

(Core :: Networking: HTTP, enhancement)

Description

[Patrick McManus [:mcmanus]]

the various flavors of xpcom getService() are actually quite expensive.

There are a few uncached services that are used in very common http channel paths..  (the error ones aren't so important). I have a patch in progress for 4 or 5 of them to be cached in httpHandler.

Comment 1

[Patrick McManus [:mcmanus]]

Attachment: Bug 1362498 - cache some networking mozilla::services

Review commit: https://reviewboard.mozilla.org/r/136614/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/136614/

Comment 2

[Patrick McManus [:mcmanus]]

Comment on attachment 8864957 [details]
Bug 1362498 - cache some networking mozilla::services

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136614/diff/1-2/

Comment 3

[Patrick McManus [:mcmanus]]

Comment on attachment 8864957 [details]
Bug 1362498 - cache some networking mozilla::services

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136614/diff/2-3/

Comment 4

[Patrick McManus [:mcmanus]]

Comment on attachment 8864957 [details]
Bug 1362498 - cache some networking mozilla::services

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136614/diff/3-4/

Comment 5

[Patrick McManus [:mcmanus]]

Comment on attachment 8864957 [details]
Bug 1362498 - cache some networking mozilla::services

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136614/diff/4-5/

Comment 6

[Patrick McManus [:mcmanus]]

Comment on attachment 8864957 [details]
Bug 1362498 - cache some networking mozilla::services

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136614/diff/5-6/

Comment 7

[Patrick McManus [:mcmanus]]

Comment on attachment 8864957 [details]
Bug 1362498 - cache some networking mozilla::services

https://reviewboard.mozilla.org/r/136614/#review139804

::: netwerk/base/nsProtocolProxyService.cpp:1212
(Diff revision 6)
>  
>      nsCOMPtr<nsIProxyInfo> pi;
>      bool usePACThread;
>  
> +    // adapt to realtime changes in the system proxy service
> +    if (mProxyConfig == PROXYCONFIG_SYSTEM) {

afaict this is a latent bug where some bits of priv'd chrome change the nsISystemProxySettings service long after init and expected that to be picked up.. our old late binding happened to do it for one value, but would not have worked if it kept being changed.

Comment 8

[Patrick McManus [:mcmanus]]

Comment on attachment 8864957 [details]
Bug 1362498 - cache some networking mozilla::services

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136614/diff/6-7/

Comment 9

[Patrick McManus [:mcmanus]]

Comment on attachment 8864957 [details]
Bug 1362498 - cache some networking mozilla::services

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136614/diff/7-8/

Comment 10

[Patrick McManus [:mcmanus]]

Comment on attachment 8864957 [details]
Bug 1362498 - cache some networking mozilla::services

https://reviewboard.mozilla.org/r/136614/#review139868

::: toolkit/components/url-classifier/nsUrlClassifierDBService.cpp:2304
(Diff revision 8)
>      }
>    } else if (!strcmp(aTopic, "quit-application")) {
>      // Tell the update thread to finish as soon as possible.
>      gShuttingDownThread = true;
> -  } else if (!strcmp(aTopic, "profile-before-change")) {
> +  } else if (!strcmp(aTopic, "profile-before-change") ||
> +             !strcmp(aTopic, "xpcom-will-shutdown")) {

this service required application events that aren't sent during xpcshell tests.. that worked before becuase the xpcshell tests never touched this service until the httphandler picked up a reference to it. The double shutdown() here is harmless (and short circuited).

Comment 11

[Patrick McManus [:mcmanus]]

Comment on attachment 8864957 [details]
Bug 1362498 - cache some networking mozilla::services

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136614/diff/8-9/

Comment 12

[Patrick McManus [:mcmanus]]

Comment on attachment 8864957 [details]
Bug 1362498 - cache some networking mozilla::services

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136614/diff/9-10/

Comment 13

[Patrick McManus [:mcmanus]]

note to self - c1 c2 c3 keep having try troubles with generic timeouts.. maybe related to https://bugzilla.mozilla.org/show_bug.cgi?id=1260312

Comment 14

[Patrick McManus [:mcmanus]]

Comment on attachment 8864957 [details]
Bug 1362498 - cache some networking mozilla::services

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136614/diff/10-11/

Comment 15

[Patrick McManus [:mcmanus]]

Comment on attachment 8864957 [details]
Bug 1362498 - cache some networking mozilla::services

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136614/diff/11-12/

Comment 16

[Daniel Stenberg [:bagder]]

Comment on attachment 8864957 [details]
Bug 1362498 - cache some networking mozilla::services

https://reviewboard.mozilla.org/r/136614/#review141510

Clean and straight-forward!

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/89150ec24a6f
cache some networking mozilla::services r=bagder

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/89150ec24a6f