From a20307b0173fc666d1b898aca143f03a75e3f35b Mon Sep 17 00:00:00 2001 From: Wes Kocher Date: Fri, 14 Mar 2014 15:27:21 -0700 Subject: [PATCH] Backed out changeset 2bdd2d042847 (bug 972577) for non-unified bustage on a CLOSED TREE --HG-- rename : xpcom/build/nsWindowsDllInterceptor.h => toolkit/xre/nsWindowsDllInterceptor.h rename : xpcom/build/IOInterposer.cpp => tools/profiler/IOInterposer.cpp rename : xpcom/build/IOInterposer.h => tools/profiler/IOInterposer.h rename : xpcom/build/NSPRInterposer.cpp => tools/profiler/NSPRInterposer.cpp rename : xpcom/build/NSPRInterposer.h => tools/profiler/NSPRInterposer.h --- storage/src/TelemetryVFS.cpp | 7 +-- toolkit/components/telemetry/Telemetry.cpp | 6 ++- toolkit/xre/moz.build | 1 + toolkit/xre/nsAppRunner.cpp | 5 -- .../xre}/nsWindowsDllInterceptor.h | 0 .../build => tools/profiler}/IOInterposer.cpp | 50 ++----------------- .../build => tools/profiler}/IOInterposer.h | 32 +----------- .../profiler}/NSPRInterposer.cpp | 0 .../build => tools/profiler}/NSPRInterposer.h | 0 .../profiler/ProfilerIOInterposeObserver.cpp | 4 -- tools/profiler/moz.build | 6 +++ tools/profiler/platform.cpp | 18 ++++++- widget/windows/winrt/MetroApp.cpp | 4 +- xpcom/build/moz.build | 11 ---- xpcom/build/nsXPComInit.cpp | 7 +++ 15 files changed, 44 insertions(+), 107 deletions(-) rename {xpcom/build => toolkit/xre}/nsWindowsDllInterceptor.h (100%) rename {xpcom/build => tools/profiler}/IOInterposer.cpp (89%) rename {xpcom/build => tools/profiler}/IOInterposer.h (91%) rename {xpcom/build => tools/profiler}/NSPRInterposer.cpp (100%) rename {xpcom/build => tools/profiler}/NSPRInterposer.h (100%) diff --git a/storage/src/TelemetryVFS.cpp b/storage/src/TelemetryVFS.cpp index bcb4fd9fd025..423788c90526 100644 --- a/storage/src/TelemetryVFS.cpp +++ b/storage/src/TelemetryVFS.cpp @@ -107,10 +107,7 @@ public: Telemetry::AccumulateTimeDelta(static_cast(id + mainThread), start, end); } - // We don't report SQLite I/O on Windows because we have a comprehensive - // mechanism for intercepting I/O on that platform that captures a superset - // of the data captured here. -#if defined(MOZ_ENABLE_PROFILER_SPS) && !defined(XP_WIN) +#ifdef MOZ_ENABLE_PROFILER_SPS if (IOInterposer::IsObservedOperation(op)) { const char* main_ref = "sqlite-mainthread"; const char* other_ref = "sqlite-otherthread"; @@ -121,7 +118,7 @@ public: // Report observation IOInterposer::Report(ob); } -#endif /* defined(MOZ_ENABLE_PROFILER_SPS) && !defined(XP_WIN) */ +#endif /* MOZ_ENABLE_PROFILER_SPS */ } private: diff --git a/toolkit/components/telemetry/Telemetry.cpp b/toolkit/components/telemetry/Telemetry.cpp index a90a42aeff70..c446ad57973c 100644 --- a/toolkit/components/telemetry/Telemetry.cpp +++ b/toolkit/components/telemetry/Telemetry.cpp @@ -386,7 +386,7 @@ void TelemetryIOInterposeObserver::AddPath(const nsAString& aPath, void TelemetryIOInterposeObserver::Observe(Observation& aOb) { // We only report main-thread I/O - if (!IsMainThread()) { + if (!NS_IsMainThread()) { return; } @@ -2968,6 +2968,10 @@ InitIOReporting(nsIFile* aXreDir) return; } + // Initialize IO interposing + IOInterposer::Init(); + InitPoisonIOInterposer(); + sTelemetryIOObserver = new TelemetryIOInterposeObserver(aXreDir); IOInterposer::Register(IOInterposeObserver::OpAll, sTelemetryIOObserver); } diff --git a/toolkit/xre/moz.build b/toolkit/xre/moz.build index a13a9ef3bfc1..347146296a9a 100644 --- a/toolkit/xre/moz.build +++ b/toolkit/xre/moz.build @@ -24,6 +24,7 @@ if CONFIG['MOZ_INSTRUMENT_EVENT_LOOP']: EXPORTS += ['EventTracer.h'] if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'windows': + EXPORTS += ['nsWindowsDllInterceptor.h'] UNIFIED_SOURCES += [ 'nsNativeAppSupportWin.cpp', ] diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp index b81ff321da63..66bd32d757cc 100644 --- a/toolkit/xre/nsAppRunner.cpp +++ b/toolkit/xre/nsAppRunner.cpp @@ -14,7 +14,6 @@ #include "mozilla/ArrayUtils.h" #include "mozilla/Attributes.h" -#include "mozilla/IOInterposer.h" #include "mozilla/Likely.h" #include "mozilla/Poison.h" #include "mozilla/Preferences.h" @@ -4026,8 +4025,6 @@ XREMain::XRE_main(int argc, char* argv[], const nsXREAppData* aAppData) GeckoProfilerInitRAII profilerGuard(&aLocal); PROFILER_LABEL("Startup", "XRE_Main"); - mozilla::IOInterposerInit ioInterposerGuard; - nsresult rv = NS_OK; gArgc = argc; @@ -4231,8 +4228,6 @@ XRE_mainMetro(int argc, char* argv[], const nsXREAppData* aAppData) GeckoProfilerInitRAII profilerGuard(&aLocal); PROFILER_LABEL("Startup", "XRE_Main"); - mozilla::IOInterposerInit ioInterposerGuard; - nsresult rv = NS_OK; xreMainPtr = new XREMain(); diff --git a/xpcom/build/nsWindowsDllInterceptor.h b/toolkit/xre/nsWindowsDllInterceptor.h similarity index 100% rename from xpcom/build/nsWindowsDllInterceptor.h rename to toolkit/xre/nsWindowsDllInterceptor.h diff --git a/xpcom/build/IOInterposer.cpp b/tools/profiler/IOInterposer.cpp similarity index 89% rename from xpcom/build/IOInterposer.cpp rename to tools/profiler/IOInterposer.cpp index b825987a1048..9f6ed9cd2e56 100644 --- a/xpcom/build/IOInterposer.cpp +++ b/tools/profiler/IOInterposer.cpp @@ -9,12 +9,6 @@ #include "mozilla/Mutex.h" #include "mozilla/StaticPtr.h" -#include "mozilla/ThreadLocal.h" -#if !defined(XP_WIN) -#include "NSPRInterposer.h" -#endif // !defined(XP_WIN) -#include "nsXULAppAPI.h" -#include "PoisonIOInterposer.h" using namespace mozilla; @@ -76,7 +70,6 @@ public: // List of observers registered static StaticAutoPtr sObserverLists; -static ThreadLocal sIsMainThread; /** Find if a vector contains a specific element */ template @@ -142,30 +135,6 @@ IOInterposeObserver::Operation IOInterposer::sObservedOperations = } sObserverLists = new ObserverLists(); sObservedOperations = IOInterposeObserver::OpNone; - if (sIsMainThread.init()) { -#if defined(XP_WIN) - bool isMainThread = XRE_GetWindowsEnvironment() != - WindowsEnvironmentType_Metro; -#else - bool isMainThread = true; -#endif - sIsMainThread.set(isMainThread); - } - // Now we initialize the various interposers depending on platform -#if defined(XP_WIN) || defined(XP_MACOSX) - InitPoisonIOInterposer(); -#endif - // We don't hook NSPR on Windows because PoisonIOInterposer captures a - // superset of the former's events. -#if !defined(XP_WIN) - InitNSPRIOInterposing(); -#endif -} - -/* static */ bool -IOInterposeObserver::IsMainThread() -{ - return sIsMainThread.initialized() && sIsMainThread.get(); } /* static */ void IOInterposer::Clear() @@ -257,6 +226,8 @@ IOInterposeObserver::IsMainThread() /* static */ void IOInterposer::Register(IOInterposeObserver::Operation aOp, IOInterposeObserver* aObserver) { + // IOInterposer::Init most be called before this method + MOZ_ASSERT(sObserverLists); // We should never register nullptr as observer MOZ_ASSERT(aObserver); if (!sObserverLists || !aObserver) { @@ -301,6 +272,8 @@ IOInterposeObserver::IsMainThread() /* static */ void IOInterposer::Unregister(IOInterposeObserver::Operation aOp, IOInterposeObserver* aObserver) { + // IOInterposer::Init most be called before this method. + MOZ_ASSERT(sObserverLists); if (!sObserverLists) { return; } @@ -351,18 +324,3 @@ IOInterposeObserver::IsMainThread() } } } - -/* static */ void -IOInterposer::RegisterCurrentThread(bool aIsMainThread) -{ - // Right now this is a no-op unless we're running on Metro. - // More cross-platform stuff will be added in the near future, stay tuned! -#if defined(XP_WIN) - if (XRE_GetWindowsEnvironment() != WindowsEnvironmentType_Metro || - !sIsMainThread.initialized()) { - return; - } - sIsMainThread.set(aIsMainThread); -#endif -} - diff --git a/xpcom/build/IOInterposer.h b/tools/profiler/IOInterposer.h similarity index 91% rename from xpcom/build/IOInterposer.h rename to tools/profiler/IOInterposer.h index e80ddc9a810a..10ac05c6a009 100644 --- a/xpcom/build/IOInterposer.h +++ b/tools/profiler/IOInterposer.h @@ -136,14 +136,6 @@ public: virtual ~IOInterposeObserver() { } - -protected: - /** - * We don't use NS_IsMainThread() because we need to be able to determine the - * main thread outside of XPCOM Initialization. IOInterposer observers should - * call this function instead. - */ - static bool IsMainThread(); }; #ifdef MOZ_ENABLE_PROFILER_SPS @@ -246,19 +238,10 @@ public: * didn't register for them all. * I.e. IOInterposer::Unregister(IOInterposeObserver::OpAll, aObserver) * - * Remark: Init() must be called before observers are unregistered. + * Remark: Init() must be called before observers are unregistered */ static void Unregister(IOInterposeObserver::Operation aOp, IOInterposeObserver* aObserver); - - /** - * Registers the current thread with the IOInterposer. - * - * @param aIsMainThread true if IOInterposer should treat the current thread - * as the main thread. - */ - static void - RegisterCurrentThread(bool aIsMainThread = false); }; #else /* MOZ_ENABLE_PROFILER_SPS */ @@ -277,23 +260,10 @@ public: static inline bool IsObservedOperation(IOInterposeObserver::Operation aOp) { return false; } - static inline void RegisterCurrentThread(bool) {} }; #endif /* MOZ_ENABLE_PROFILER_SPS */ -class IOInterposerInit -{ -public: - IOInterposerInit() - { - IOInterposer::Init(); - } - - // No destructor needed at the moment -- this stuff stays active for the - // life of the process. This may change in the future. -}; - } // namespace mozilla #endif // mozilla_IOInterposer_h diff --git a/xpcom/build/NSPRInterposer.cpp b/tools/profiler/NSPRInterposer.cpp similarity index 100% rename from xpcom/build/NSPRInterposer.cpp rename to tools/profiler/NSPRInterposer.cpp diff --git a/xpcom/build/NSPRInterposer.h b/tools/profiler/NSPRInterposer.h similarity index 100% rename from xpcom/build/NSPRInterposer.h rename to tools/profiler/NSPRInterposer.h diff --git a/tools/profiler/ProfilerIOInterposeObserver.cpp b/tools/profiler/ProfilerIOInterposeObserver.cpp index 757189c7f159..73788fc2a3ea 100644 --- a/tools/profiler/ProfilerIOInterposeObserver.cpp +++ b/tools/profiler/ProfilerIOInterposeObserver.cpp @@ -10,10 +10,6 @@ using namespace mozilla; void ProfilerIOInterposeObserver::Observe(Observation& aObservation) { - if (!IsMainThread()) { - return; - } - const char* str = nullptr; switch (aObservation.ObservedOperation()) { diff --git a/tools/profiler/moz.build b/tools/profiler/moz.build index 4710841e7be9..ff009ecb2ebb 100644 --- a/tools/profiler/moz.build +++ b/tools/profiler/moz.build @@ -24,8 +24,10 @@ if CONFIG['MOZ_ENABLE_PROFILER_SPS']: ] UNIFIED_SOURCES += [ 'BreakpadSampler.cpp', + 'IOInterposer.cpp', 'JSCustomObjectBuilder.cpp', 'JSObjectBuilder.cpp', + 'NSPRInterposer.cpp', 'nsProfiler.cpp', 'nsProfilerFactory.cpp', 'platform.cpp', @@ -88,4 +90,8 @@ EXPORTS += [ 'GeckoProfiler.h', ] +EXPORTS.mozilla += [ + 'IOInterposer.h', +] + XPCSHELL_TESTS_MANIFESTS += ['tests/xpcshell.ini'] diff --git a/tools/profiler/platform.cpp b/tools/profiler/platform.cpp index 8a4f29e85631..cb339ac63d2f 100644 --- a/tools/profiler/platform.cpp +++ b/tools/profiler/platform.cpp @@ -7,6 +7,8 @@ #include #include +#include "IOInterposer.h" +#include "NSPRInterposer.h" #include "ProfilerIOInterposeObserver.h" #include "platform.h" #include "PlatformMacros.h" @@ -479,6 +481,11 @@ void mozilla_sampler_init(void* stackTop) // platform specific initialization OS::Startup(); + // Initialize I/O interposing + mozilla::IOInterposer::Init(); + // Initialize NSPR I/O Interposing + mozilla::InitNSPRIOInterposing(); + // We can't open pref so we use an environment variable // to know if we should trigger the profiler on startup // NOTE: Default @@ -526,6 +533,16 @@ void mozilla_sampler_shutdown() profiler_stop(); + // Unregister IO interpose observer + mozilla::IOInterposer::Unregister(mozilla::IOInterposeObserver::OpAll, + sInterposeObserver); + // mozilla_sampler_shutdown is only called at shutdown, and late-write checks + // might need the IO interposer, so we don't clear it. Don't worry it's + // designed not to report leaks. + // mozilla::IOInterposer::Clear(); + mozilla::ClearNSPRIOInterposing(); + sInterposeObserver = nullptr; + Sampler::Shutdown(); // We can't delete the Stack because we can be between a @@ -724,7 +741,6 @@ void mozilla_sampler_stop() mozilla::IOInterposer::Unregister(mozilla::IOInterposeObserver::OpAll, sInterposeObserver); - sInterposeObserver = nullptr; sIsProfiling = false; diff --git a/widget/windows/winrt/MetroApp.cpp b/widget/windows/winrt/MetroApp.cpp index 8b2c0442c7e1..f41f2f9b1d0a 100644 --- a/widget/windows/winrt/MetroApp.cpp +++ b/widget/windows/winrt/MetroApp.cpp @@ -5,7 +5,6 @@ #include "MetroApp.h" #include "MetroWidget.h" -#include "mozilla/IOInterposer.h" #include "mozilla/widget/AudioSession.h" #include "nsIRunnable.h" #include "MetroUtils.h" @@ -77,11 +76,10 @@ MetroApp::Run() LogThread(); // Name this thread for debugging and register it with the profiler - // and IOInterposer as the main gecko thread. + // as the main gecko thread. char aLocal; PR_SetCurrentThreadName(gGeckoThreadName); profiler_register_thread(gGeckoThreadName, &aLocal); - IOInterposer::RegisterCurrentThread(true); HRESULT hr; hr = sCoreApp->add_Suspending(Callback<__FIEventHandler_1_Windows__CApplicationModel__CSuspendingEventArgs_t>( diff --git a/xpcom/build/moz.build b/xpcom/build/moz.build index d8738f79270c..3887e908eac7 100644 --- a/xpcom/build/moz.build +++ b/xpcom/build/moz.build @@ -15,7 +15,6 @@ EXPORTS += [ EXPORTS.mozilla += [ 'FileLocation.h', - 'IOInterposer.h', 'LateWriteChecks.h', 'Omnijar.h', 'PoisonIOInterposer.h', @@ -25,7 +24,6 @@ EXPORTS.mozilla += [ ] if CONFIG['OS_ARCH'] == 'WINNT': - EXPORTS += ['nsWindowsDllInterceptor.h'] EXPORTS.mozilla += ['perfprobe.h'] SOURCES += ['perfprobe.cpp'] if CONFIG['MOZ_ENABLE_PROFILER_SPS']: @@ -57,15 +55,6 @@ UNIFIED_SOURCES += [ 'Services.cpp', ] -if CONFIG['MOZ_ENABLE_PROFILER_SPS']: - SOURCES += [ - 'IOInterposer.cpp', - ] - if CONFIG['OS_ARCH'] != 'WINNT': - SOURCES += [ - 'NSPRInterposer.cpp', - ] - # FileLocation.cpp and Omnijar.cpp cannot be built in unified mode because they # use plarena.h. SOURCES += [ diff --git a/xpcom/build/nsXPComInit.cpp b/xpcom/build/nsXPComInit.cpp index 9b257f1ab837..2e7aecbd67dc 100644 --- a/xpcom/build/nsXPComInit.cpp +++ b/xpcom/build/nsXPComInit.cpp @@ -110,6 +110,8 @@ extern nsresult nsStringInputStreamConstructor(nsISupports *, REFNSIID, void **) #include "nsChromeRegistry.h" #include "nsChromeProtocolHandler.h" +#include "mozilla/IOInterposer.h" +#include "mozilla/PoisonIOInterposer.h" #include "mozilla/LateWriteChecks.h" #include "mozilla/scache/StartupCache.h" @@ -967,6 +969,11 @@ ShutdownXPCOM(nsIServiceManager* servMgr) PROFILER_MARKER("Shutdown xpcom"); // If we are doing any shutdown checks, poison writes. if (gShutdownChecks != SCM_NOTHING) { + // Calling InitIOInterposer or InitPoisonIOInterposer twice doesn't + // cause any problems, they'll safely abort the initialization on their + // own initiative. + mozilla::IOInterposer::Init(); + mozilla::InitPoisonIOInterposer(); #ifdef XP_MACOSX mozilla::OnlyReportDirtyWrites(); #endif /* XP_MACOSX */