Closed Bug 1501404 Opened Last year Closed Last year

Remove some XPCOM registrations from Necko

Categories

(Core :: Networking, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: ehsan, Assigned: ehsan)

References

(Blocks 1 open bug)

Details

Attachments

(8 files)

No description provided.
Blocks: 1477576
Pushed by eakhgari@mozilla.com:
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
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
Hrm.  I didn't expect that to happen.

The link in comment 11 shows nothing for me.  Was this Windows only?
Flags: needinfo?(igoldan)
Oh, nm.  Now it works.
Flags: needinfo?(igoldan)
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.)
Flags: needinfo?(nfroyd)
(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.
Flags: needinfo?(nfroyd)
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.
See Also: → 1502372
You need to log in before you can comment on or make changes to this bug.