1501404 - Remove some XPCOM registrations from Necko

Status: RESOLVED FIXED

(Core :: Networking, enhancement)

Comment 1

[(no longer active)]

Attachment: Bug 1501404 - Part 1: Remove the XPCOM registration for RequestContextService

Comment 2

[(no longer active)]

Attachment: Bug 1501404 - Part 2: Remove the XPCOM registration for nsRequestObserverProxy

Depends on D9570

Comment 3

[(no longer active)]

Attachment: Bug 1501404 - Part 3: Remove the unused contract ID for nsIProxyAutoConfig

Depends on D9571

Comment 4

[(no longer active)]

Attachment: Bug 1501404 - Part 4: Remove the unused contract ID for nsIHttpPushListener

Depends on D9572

Comment 5

[(no longer active)]

Attachment: Bug 1501404 - Part 5: Remove the XPCOM registration for nsSocketProviderService

Depends on D9573

Comment 6

[(no longer active)]

Attachment: Bug 1501404 - Part 6: Remove the XPCOM registrations for socket provider classes

Depends on D9574

Comment 7

[(no longer active)]

Attachment: Bug 1501404 - Part 7: Remove the XPCOM registration for nsSyncStreamListener

Depends on D9575

Comment 8

[(no longer active)]

Attachment: Bug 1501404 - Part 8: Remove the XPCOM registration for NamedPipeService

Depends on D9576

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/81f3d1e42395
Part 1: Remove the XPCOM registration for RequestContextService r=valentin
https://hg.mozilla.org/integration/autoland/rev/8711f0c4d51e
Part 2: Remove the XPCOM registration for nsRequestObserverProxy r=valentin
https://hg.mozilla.org/integration/autoland/rev/6af22b13b62a
Part 3: Remove the unused contract ID for nsIProxyAutoConfig r=valentin
https://hg.mozilla.org/integration/autoland/rev/361c06422c19
Part 4: Remove the unused contract ID for nsIHttpPushListener r=valentin
https://hg.mozilla.org/integration/autoland/rev/79795a6721cc
Part 5: Remove the XPCOM registration for nsSocketProviderService r=valentin
https://hg.mozilla.org/integration/autoland/rev/1766ed669623
Part 6: Remove the XPCOM registrations for socket provider classes r=valentin
https://hg.mozilla.org/integration/autoland/rev/89474459aeb8
Part 7: Remove the XPCOM registration for nsSyncStreamListener r=valentin
https://hg.mozilla.org/integration/autoland/rev/d826438ea26d
Part 8: Remove the XPCOM registration for NamedPipeService r=valentin

Comment 10

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/81f3d1e42395
https://hg.mozilla.org/mozilla-central/rev/8711f0c4d51e
https://hg.mozilla.org/mozilla-central/rev/6af22b13b62a
https://hg.mozilla.org/mozilla-central/rev/361c06422c19
https://hg.mozilla.org/mozilla-central/rev/79795a6721cc
https://hg.mozilla.org/mozilla-central/rev/1766ed669623
https://hg.mozilla.org/mozilla-central/rev/89474459aeb8
https://hg.mozilla.org/mozilla-central/rev/d826438ea26d

Comment 11

[Ionuț Goldan [:igoldan]]

This caused some build metrics regressions:

== Change summary for alert #17067 (as of Tue, 23 Oct 2018 18:33:12 GMT) ==

Regressions:

  0.4%  compiler_metrics num_static_constructors windows2012-64 debug msvc     905.00 -> 908.42
  0.4%  compiler_metrics num_static_constructors windows2012-32 debug msvc     907.00 -> 910.42

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=17067

Comment 12

[(no longer active)]

Hrm.  I didn't expect that to happen.

The link in comment 11 shows nothing for me.  Was this Windows only?

Comment 13

[(no longer active)]

Oh, nm.  Now it works.

Comment 14

[(no longer active)]

Nathan, I was always under the impression that StaticRefPtr helps us not get a static constructor, and I used that class in all of the patches here when I created singleton pointers to hold singleton service pointers.  Is my impression here wrong?  Have you seen a similar issue elsewhere in the past by any chance?

(Note that in a couple of patches, part 7 and 8, I made the StaticRefPtr's a static class member, in other cases a static global variable.)

Comment 15

[Nathan Froyd [:froydnj]]

(In reply to :Ehsan Akhgari from comment #14)
> Nathan, I was always under the impression that StaticRefPtr helps us not get
> a static constructor, and I used that class in all of the patches here when
> I created singleton pointers to hold singleton service pointers.  Is my
> impression here wrong?  Have you seen a similar issue elsewhere in the past
> by any chance?

Your understanding matches my understanding.  But apparently not MSVC's understanding?

froydnj@hawkeye:/opt/build/froydnj/tmp$ diff -u <(grep  'dynamic initializer' xul.pdb/ACAD63CD15304A759CC5243B8755D8FA1/xul.sym | f 10 |sort) <(grep 'dynamic initializer' xul.pdb/C65E10F4D3C54128870F8DE05B7B57A01/xul.sym | f 10 | sort)
--- /dev/fd/63	2018-10-24 11:42:45.058821497 -0400
+++ /dev/fd/62	2018-10-24 11:42:45.058821497 -0400
@@ -299,6 +299,7 @@
 'gSharedMap''()
 'gSharedScriptableHelperForJSIID''()
 'gSHistoryList''()
+'gSingleton''()
 'gStaticReporter''()
 'gStorageActivityService''()
 'gStringStats''()
@@ -516,6 +517,7 @@
 'mozilla::net::CacheObserver::sHashStatsReported''()
 'mozilla::net::CacheObserver::sSanitizeOnShutdown''()
 'mozilla::net::ExtensionProtocolHandler::sSingleton''()
+'mozilla::net::NamedPipeService::gSingleton''()
 'mozilla::net::nsWSAdmissionManager::sLock''()
 'mozilla::Omnijar::sOuterReader''()
 'mozilla::Omnijar::sPath''()
@@ -606,6 +608,7 @@
 'nsNameSpaceManager::sInstance''()
 'nsPluginDestroyRunnable::sRunnableList''()
 'nsPluginHost::sInst''()
+'nsSocketProviderService::gSingleton''()
 'nsSound::sInstance''()
 'nsStyleList::sInitialQuotes''()
 'nsSVGViewBox::sSVGAnimatedRectTearoffTable''()

MSVC is creating static constructors anyway, regardless of global-scope or static class members. :(

Oh, but these are debug builds?  Whatever, we don't care: we have explicit checking in debug builds which would explicitly trigger static constructors:

https://searchfox.org/mozilla-central/source/xpcom/base/StaticPtr.h#41-58

Now, as to why these are MSVC-only...no idea.

Comment 16

[(no longer active)]

Wait a second, these are *MSVC*?!?!  I stopped caring at the 'S'.  Didn't even get to the 'd' in 'debug'.  ;-)

Sorry to have wasted your time, Nathan.