Closed Bug 1501404 Opened 6 years ago Closed 6 years ago

Remove some XPCOM registrations from Necko

Categories

(Core :: Networking, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)

References

(Blocks 1 open bug)

Details

Attachments

(8 files)

No description provided.
Depends on D9576
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.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: