diff options
author | Daniel Baumann <daniel.baumann@progress-linux.org> | 2024-06-12 05:35:29 +0000 |
---|---|---|
committer | Daniel Baumann <daniel.baumann@progress-linux.org> | 2024-06-12 05:35:29 +0000 |
commit | 59203c63bb777a3bacec32fb8830fba33540e809 (patch) | |
tree | 58298e711c0ff0575818c30485b44a2f21bf28a0 /tools/profiler | |
parent | Adding upstream version 126.0.1. (diff) | |
download | firefox-59203c63bb777a3bacec32fb8830fba33540e809.tar.xz firefox-59203c63bb777a3bacec32fb8830fba33540e809.zip |
Adding upstream version 127.0.upstream/127.0
Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>
Diffstat (limited to 'tools/profiler')
-rw-r--r-- | tools/profiler/core/memory_hooks.cpp | 7 | ||||
-rw-r--r-- | tools/profiler/core/memory_hooks.h | 1 | ||||
-rw-r--r-- | tools/profiler/core/platform.cpp | 320 | ||||
-rw-r--r-- | tools/profiler/docs/code-overview.rst | 2 | ||||
-rw-r--r-- | tools/profiler/docs/index.rst | 2 | ||||
-rw-r--r-- | tools/profiler/docs/instrumenting-android.rst | 102 | ||||
-rw-r--r-- | tools/profiler/docs/instrumenting-javascript.rst | 5 | ||||
-rw-r--r-- | tools/profiler/docs/instrumenting-rust.rst | 5 | ||||
-rw-r--r-- | tools/profiler/docs/markers-guide.rst | 5 | ||||
-rw-r--r-- | tools/profiler/public/ProfilerControl.h | 3 | ||||
-rw-r--r-- | tools/profiler/public/ProfilerCounts.h | 30 | ||||
-rw-r--r-- | tools/profiler/public/ProfilerState.h | 11 | ||||
-rw-r--r-- | tools/profiler/tests/gtest/moz.build | 3 | ||||
-rw-r--r-- | tools/profiler/tests/xpcshell/test_feature_posix_signals.js | 85 | ||||
-rw-r--r-- | tools/profiler/tests/xpcshell/xpcshell.toml | 6 |
15 files changed, 496 insertions, 91 deletions
diff --git a/tools/profiler/core/memory_hooks.cpp b/tools/profiler/core/memory_hooks.cpp index e13a109aab..5167d15d46 100644 --- a/tools/profiler/core/memory_hooks.cpp +++ b/tools/profiler/core/memory_hooks.cpp @@ -584,6 +584,7 @@ BaseProfilerCount* install_memory_hooks() { ThreadIntercept::Init(); } else { sCounter->Clear(); + sCounter->Register(); } jemalloc_replace_dynamic(replace_init); return sCounter; @@ -635,4 +636,10 @@ void disable_native_allocations() { } } +void unregister_memory_counter() { + if (sCounter) { + sCounter->Unregister(); + } +} + } // namespace mozilla::profiler diff --git a/tools/profiler/core/memory_hooks.h b/tools/profiler/core/memory_hooks.h index a6ace771dd..4f59b21136 100644 --- a/tools/profiler/core/memory_hooks.h +++ b/tools/profiler/core/memory_hooks.h @@ -17,6 +17,7 @@ BaseProfilerCount* install_memory_hooks(); void remove_memory_hooks(); void enable_native_allocations(); void disable_native_allocations(); +void unregister_memory_counter(); } // namespace profiler } // namespace mozilla diff --git a/tools/profiler/core/platform.cpp b/tools/profiler/core/platform.cpp index 6b7e318f80..4641587a9a 100644 --- a/tools/profiler/core/platform.cpp +++ b/tools/profiler/core/platform.cpp @@ -114,18 +114,32 @@ #include <string_view> #include <type_traits> +// To simplify other code in this file, define a helper definition to avoid +// repeating the same preprocessor checks. + // The signals that we use to control the profiler conflict with the signals -// used to control the code coverage tool. Therefore, if coverage is enabled, we -// need to disable our own signal handling mechanisms. +// used to control the code coverage tool. Therefore, if coverage is enabled, +// we need to disable our own signal handling mechanisms. #ifndef MOZ_CODE_COVERAGE # ifdef XP_WIN // TODO: Add support for windows "signal"-like behaviour. See Bug 1867328. +# elif defined(GP_OS_darwin) || defined(GP_OS_linux) || \ + defined(GP_OS_android) || defined(GP_OS_freebsd) +// Specify the specific platforms that we want to support +# define GECKO_PROFILER_ASYNC_POSIX_SIGNAL_CONTROL 1 # else -# include <signal.h> -# include <unistd.h> +// No support on this unknown platform! # endif #endif +// We need some extra includes if we're supporting async posix signals +#if defined(GECKO_PROFILER_ASYNC_POSIX_SIGNAL_CONTROL) +# include <signal.h> +# include <fcntl.h> +# include <unistd.h> +# include <errno.h> +#endif + #if defined(GP_OS_android) # include "JavaExceptions.h" # include "mozilla/java/GeckoJavaSamplerNatives.h" @@ -250,11 +264,28 @@ ProfileChunkedBuffer& profiler_get_core_buffer() { return sProfileChunkedBuffer; } -mozilla::Atomic<int, mozilla::MemoryOrdering::Relaxed> gSkipSampling; +#if defined(GECKO_PROFILER_ASYNC_POSIX_SIGNAL_CONTROL) +// Control character to start the profiler ('g' for "go"!) +static const char sAsyncSignalControlCharStart = 'g'; +// Control character to stop the profiler ('s' for "stop"!) +static const char sAsyncSignalControlCharStop = 's'; + +// This is a file descriptor that is the "write" end of the POSIX pipe that we +// use to start the profiler. It is written to in profiler_start_signal_handler +// and read from in AsyncSignalControlThread +static mozilla::Atomic<int, mozilla::MemoryOrdering::Relaxed> + sAsyncSignalControlWriteFd(-1); // Atomic flag to stop the profiler from within the sampling loop mozilla::Atomic<bool, mozilla::MemoryOrdering::Relaxed> gStopAndDumpFromSignal( false); +#endif + +// Forward declare the function to call when we need to dump + stop from within +// the async control thread +void profiler_dump_and_stop(); + +mozilla::Atomic<int, mozilla::MemoryOrdering::Relaxed> gSkipSampling; #if defined(GP_OS_android) class GeckoJavaSampler @@ -380,6 +411,7 @@ static uint32_t AvailableFeatures() { } #else // The memory hooks are not available. + ProfilerFeature::ClearMemory(features); ProfilerFeature::ClearNativeAllocations(features); #endif @@ -506,6 +538,143 @@ static constexpr size_t MAX_JS_FRAMES = using JsFrame = mozilla::profiler::ThreadRegistrationData::JsFrame; using JsFrameBuffer = mozilla::profiler::ThreadRegistrationData::JsFrameBuffer; +#if defined(GECKO_PROFILER_ASYNC_POSIX_SIGNAL_CONTROL) +// Forward declare this, so we can call it from the constructor. +static void* AsyncSignalControlThreadEntry(void* aArg); + +// Define our platform specific async (posix) signal control thread here. +class AsyncSignalControlThread { + public: + AsyncSignalControlThread() : mThread() { + // Try to open a pipe for this to communicate with. If we can't do this, + // then we give up and return, as there's no point continuing without + // being able to communicate + int pipeFds[2]; + if (pipe(pipeFds)) { + LOG("Profiler AsyncSignalControlThread failed to create a pipe."); + return; + } + + // Close this pipe on calls to exec(). + fcntl(pipeFds[0], F_SETFD, FD_CLOEXEC); + fcntl(pipeFds[1], F_SETFD, FD_CLOEXEC); + + // Write the reading side to mFd, and the writing side to the global atomic + mFd = pipeFds[0]; + sAsyncSignalControlWriteFd = pipeFds[1]; + + // We don't really care about stack size, as it should be minimal, so + // leave the pthread attributes as a nullptr, i.e. choose the default. + pthread_attr_t* attr_ptr = nullptr; + if (pthread_create(&mThread, attr_ptr, AsyncSignalControlThreadEntry, + this) != 0) { + MOZ_CRASH("pthread_create failed"); + } + }; + + ~AsyncSignalControlThread() { + // Derived from code in nsDumpUtils.cpp. Comment reproduced here for + // poisterity: Close sAsyncSignalControlWriteFd /after/ setting the fd to + // -1. Otherwise we have the (admittedly far-fetched) race where we + // + // 1) close sAsyncSignalControlWriteFd + // 2) open a new fd with the same number as sAsyncSignalControlWriteFd + // had. + // 3) receive a signal, then write to the fd. + int asyncSignalControlWriteFd = sAsyncSignalControlWriteFd.exchange(-1); + // This will unblock the "read" in StartWatching. + close(asyncSignalControlWriteFd); + // Finally, exit the thread. + pthread_join(mThread, nullptr); + }; + + void Watch() { + char msg[1]; + ssize_t nread; + while (true) { + // Try reading from the pipe. This will block until something is written: + nread = read(mFd, msg, sizeof(msg)); + + if (nread == -1 && errno == EINTR) { + // nread == -1 and errno == EINTR means that `read` was interrupted + // by a signal before reading any data. This is likely because the + // profiling thread interrupted us (with SIGPROF). We can safely ignore + // this and "go around" the loop again to try and read. + continue; + } + + if (nread == -1 && errno != EINTR) { + // nread == -1 and errno != EINTR means that `read` has failed in some + // way that we can't recover from. In this case, all we can do is give + // up, and quit the watcher, as the pipe is likely broken. + LOG("Error (%d) when reading in AsyncSignalControlThread", errno); + return; + } + + if (nread == 0) { + // nread == 0 signals that the other end of the pipe has been cleanly + // closed. Close our end, and exit the reading loop. + close(mFd); + return; + } + + // If we reach here, nread != 0 and nread != -1. This means that we + // should have read at least one byte, which should be a control byte + // for the profiler. + // It *might* happen that `read` is interrupted by the sampler thread + // after successfully reading. If this occurs, read returns the number + // of bytes read. As anything other than 1 is wrong for us, we can + // always assume that we can read whatever `read` read. + MOZ_RELEASE_ASSERT(nread == 1); + + if (msg[0] == sAsyncSignalControlCharStart) { + // Start the profiler here directly, as we're on a background thread. + // set of preferences, configuration of them is TODO, see Bug 1866007 + uint32_t features = ProfilerFeature::JS | ProfilerFeature::StackWalk | + ProfilerFeature::CPUUtilization; + // as we often don't know what threads we'll care about, tell the + // profiler to profile all threads. + const char* filters[] = {"*"}; + profiler_start(PROFILER_DEFAULT_SIGHANDLE_ENTRIES, + PROFILER_DEFAULT_INTERVAL, features, filters, + MOZ_ARRAY_LENGTH(filters), 0); + } else if (msg[0] == sAsyncSignalControlCharStop) { + // Check to see whether the profiler is even running before trying to + // stop the profiler. Most other methods of stopping the profiler (i.e. + // those through nsProfiler etc) already know whether or not the + // profiler is running, so don't try and stop it if it's already + // running. Signal-stopping doesn't have this constraint, so we should + // check just in case there is a codepath followed by + // `profiler_dump_and_stop` that breaks if we stop while stopped. + if (profiler_is_active()) { + profiler_dump_and_stop(); + } + } else { + LOG("AsyncSignalControlThread recieved unknown control signal: %c", + msg[0]); + } + } + }; + + private: + // The read side of the pipe that we use to communicate from a signal handler + // to the AsyncSignalControlThread + int mFd; + + // The thread handle for the async signal control thread + // Note, that unlike the sampler thread, this is currently a posix-only + // feature. Therefore, we don't bother to have a windows equivalent - we + // just use a pthread_t + pthread_t mThread; +}; + +static void* AsyncSignalControlThreadEntry(void* aArg) { + auto* thread = static_cast<AsyncSignalControlThread*>(aArg); + thread->Watch(); + return nullptr; +} +#endif + // All functions in this file can run on multiple threads unless they have an // NS_IsMainThread() assertion. @@ -529,6 +698,10 @@ class CorePS { CorePS() : mProcessStartTime(TimeStamp::ProcessCreation()), mMaybeBandwidthCounter(nullptr) +#if defined(GECKO_PROFILER_ASYNC_POSIX_SIGNAL_CONTROL) + , + mAsyncSignalControlThread(nullptr) +#endif #ifdef USE_LUL_STACKWALK , mLul(nullptr) @@ -539,6 +712,9 @@ class CorePS { } ~CorePS() { +#if defined(GECKO_PROFILER_ASYNC_POSIX_SIGNAL_CONTROL) + delete mAsyncSignalControlThread; +#endif #ifdef USE_LUL_STACKWALK delete sInstance->mLul; delete mMaybeBandwidthCounter; @@ -684,6 +860,14 @@ class CorePS { return sInstance->mMaybeBandwidthCounter; } +#if defined(GECKO_PROFILER_ASYNC_POSIX_SIGNAL_CONTROL) + static void SetAsyncSignalControlThread( + AsyncSignalControlThread* aAsyncSignalControlThread) { + MOZ_ASSERT(sInstance); + sInstance->mAsyncSignalControlThread = aAsyncSignalControlThread; + } +#endif + private: // The singleton instance static CorePS* sInstance; @@ -701,6 +885,11 @@ class CorePS { // Non-owning pointers to all active counters Vector<BaseProfilerCount*> mCounters; +#if defined(GECKO_PROFILER_ASYNC_POSIX_SIGNAL_CONTROL) + // Background thread for communicating with async signal handlers + AsyncSignalControlThread* mAsyncSignalControlThread; +#endif + #ifdef USE_LUL_STACKWALK // LUL's state. Null prior to the first activation, non-null thereafter. // Owned by this CorePS. @@ -1139,6 +1328,11 @@ class ActivePS { #undef PS_GET_FEATURE + static bool ShouldInstallMemoryHooks(PSLockRef) { + MOZ_ASSERT(sInstance); + return ProfilerFeature::ShouldInstallMemoryHooks(sInstance->mFeatures); + } + static uint32_t JSFlags(PSLockRef aLock) { uint32_t Flags = 0; Flags |= @@ -4106,10 +4300,6 @@ static SamplerThread* NewSamplerThread(PSLockRef aLock, uint32_t aGeneration, return new SamplerThread(aLock, aGeneration, aInterval, aFeatures); } -// Forward declare the function to call when we need to dump + stop from within -// the sampler thread -void profiler_dump_and_stop(); - // This function is the sampler thread. This implementation is used for all // targets. void SamplerThread::Run() { @@ -4765,27 +4955,6 @@ void SamplerThread::Run() { scheduledSampleStart = beforeSleep + sampleInterval; SleepMicro(static_cast<uint32_t>(sampleInterval.ToMicroseconds())); } - - // Check to see if the hard-reset flag has been set to stop the profiler. - // This should only be used on the worst occasions when we need to stop the - // profiler from within the sampling thread (e.g. if the main thread is - // stuck) We need to do this here as it is outside of the scope of the lock. - // Otherwise we'll encounter a race condition where `profiler_stop` tries to - // get the lock that we already hold. We also need to wait until after we - // have carried out post sampling callbacks, as otherwise we may reach a - // situation where another part of the program is waiting for us to finish - // sampling, but we have ended early! - if (gStopAndDumpFromSignal) { - // Reset the flag in case we restart the profiler at a later point - gStopAndDumpFromSignal = false; - // dump the profile, and stop the profiler - profiler_dump_and_stop(); - // profiler_stop will try to destroy the active sampling thread. This will - // also destroy some data structures that are used further down this - // function, leading to invalid accesses. We therefore exit the function - // directly, rather than breaking from the loop. - return; - } } // End of `while` loop. We can only be here from a `break` inside the loop. @@ -5069,9 +5238,9 @@ void SamplerThread::SpyOnUnregisteredThreads() { MOZ_DEFINE_MALLOC_SIZE_OF(GeckoProfilerMallocSizeOf) -NS_IMETHODIMP -GeckoProfilerReporter::CollectReports(nsIHandleReportCallback* aHandleReport, - nsISupports* aData, bool aAnonymize) { +NS_IMETHODIMP GeckoProfilerReporter::CollectReports( + nsIHandleReportCallback* aHandleReport, nsISupports* aData, + bool aAnonymize) { MOZ_RELEASE_ASSERT(NS_IsMainThread()); size_t profSize = 0; @@ -5294,16 +5463,41 @@ static const char* get_size_suffix(const char* str) { return ptr; } -#if !defined(XP_WIN) && !defined(MOZ_CODE_COVERAGE) +#if defined(GECKO_PROFILER_ASYNC_POSIX_SIGNAL_CONTROL) +static void profiler_start_signal_handler(int signal, siginfo_t* info, + void* context) { + // Starting the profiler from a signal handler is a risky business: Both of + // the main tasks that we would like to accomplish (allocating memory, and + // starting a thread) are illegal within a UNIX signal handler. Conversely, + // we cannot dispatch to the main thread, as this may be "stuck" (why else + // would we be using a signal handler to start the profiler?). + // Instead, we have a background thread running that watches a pipe for a + // given "control" character. In this handler, we can simply write to that + // pipe to get the background thread to start the profiler for us! + // Note that `write` is async-signal safe (see signal-safety(7)): + // https://www.man7.org/linux/man-pages/man7/signal-safety.7.html + // This means that it's safe for us to call within a signal handler. + if (sAsyncSignalControlWriteFd != -1) { + char signalControlCharacter = sAsyncSignalControlCharStart; + Unused << write(sAsyncSignalControlWriteFd, &signalControlCharacter, + sizeof(signalControlCharacter)); + } +} + static void profiler_stop_signal_handler(int signal, siginfo_t* info, void* context) { - // We cannot really do any logging here, as this is a signal handler. // Signal handlers are limited in what functions they can call, for more // details see: https://man7.org/linux/man-pages/man7/signal-safety.7.html - // Writing to a file is allowed, but as signal handlers are also limited in - // how long they can run, we instead set an atomic variable to true to trigger - // the sampling thread to stop and dump the data in the profiler. - gStopAndDumpFromSignal = true; + // As we have a background thread already running for checking whether or + // not we want to start the profiler, we can re-use the same machinery to + // stop the profiler. We use the same mechanism of writing to a pipe/file + // descriptor, but with a different control character. Note that `write` is + // signal safe. + if (sAsyncSignalControlWriteFd != -1) { + char signalControlCharacter = sAsyncSignalControlCharStop; + Unused << write(sAsyncSignalControlWriteFd, &signalControlCharacter, + sizeof(signalControlCharacter)); + } } #endif @@ -5379,8 +5573,17 @@ void profiler_dump_and_stop() { profiler_stop(); } +#if defined(GECKO_PROFILER_ASYNC_POSIX_SIGNAL_CONTROL) void profiler_init_signal_handlers() { -#if !defined(XP_WIN) && !defined(MOZ_CODE_COVERAGE) + // Set a handler to start the profiler + struct sigaction prof_start_sa {}; + memset(&prof_start_sa, 0, sizeof(struct sigaction)); + prof_start_sa.sa_sigaction = profiler_start_signal_handler; + prof_start_sa.sa_flags = SA_RESTART | SA_SIGINFO; + sigemptyset(&prof_start_sa.sa_mask); + DebugOnly<int> rstart = sigaction(SIGUSR1, &prof_start_sa, nullptr); + MOZ_ASSERT(rstart == 0, "Failed to install Profiler SIGUSR1 handler"); + // Set a handler to stop the profiler struct sigaction prof_stop_sa {}; memset(&prof_stop_sa, 0, sizeof(struct sigaction)); @@ -5389,8 +5592,8 @@ void profiler_init_signal_handlers() { sigemptyset(&prof_stop_sa.sa_mask); DebugOnly<int> rstop = sigaction(SIGUSR2, &prof_stop_sa, nullptr); MOZ_ASSERT(rstop == 0, "Failed to install Profiler SIGUSR2 handler"); -#endif } +#endif void profiler_init(void* aStackTop) { LOG("profiler_init"); @@ -5444,8 +5647,14 @@ void profiler_init(void* aStackTop) { // Platform-specific initialization. PlatformInit(lock); +#if defined(GECKO_PROFILER_ASYNC_POSIX_SIGNAL_CONTROL) + // Initialise the background thread to listen for signal handler + // communication + CorePS::SetAsyncSignalControlThread(new AsyncSignalControlThread); + // Initialise the signal handlers needed to start/stop the profiler profiler_init_signal_handlers(); +#endif #if defined(GP_OS_android) if (jni::IsAvailable()) { @@ -5611,9 +5820,16 @@ void profiler_init(void* aStackTop) { profiler_mark_thread_awake(); #if defined(MOZ_REPLACE_MALLOC) && defined(MOZ_PROFILER_MEMORY) - // Start counting memory allocations (outside of lock because this may call - // profiler_add_sampled_counter which would attempt to take the lock.) - ActivePS::SetMemoryCounter(mozilla::profiler::install_memory_hooks()); + if (ProfilerFeature::ShouldInstallMemoryHooks(features)) { + // Start counting memory allocations (outside of lock because this may call + // profiler_add_sampled_counter which would attempt to take the lock.) + ActivePS::SetMemoryCounter(mozilla::profiler::install_memory_hooks()); + } else { + // Unregister the memory counter in case it was registered before. This will + // make sure that the empty memory counter from the previous profiler run is + // removed completely and we don't serialize the memory counters. + mozilla::profiler::unregister_memory_counter(); + } #endif invoke_profiler_state_change_callbacks(ProfilingState::Started); @@ -6240,9 +6456,16 @@ RefPtr<GenericPromise> profiler_start(PowerOfTwo32 aCapacity, double aInterval, } #if defined(MOZ_REPLACE_MALLOC) && defined(MOZ_PROFILER_MEMORY) - // Start counting memory allocations (outside of lock because this may call - // profiler_add_sampled_counter which would attempt to take the lock.) - ActivePS::SetMemoryCounter(mozilla::profiler::install_memory_hooks()); + if (ProfilerFeature::ShouldInstallMemoryHooks(aFeatures)) { + // Start counting memory allocations (outside of lock because this may call + // profiler_add_sampled_counter which would attempt to take the lock.) + ActivePS::SetMemoryCounter(mozilla::profiler::install_memory_hooks()); + } else { + // Unregister the memory counter in case it was registered before. This will + // make sure that the empty memory counter from the previous profiler run is + // removed completely and we don't serialize the memory counters. + mozilla::profiler::unregister_memory_counter(); + } #endif invoke_profiler_state_change_callbacks(ProfilingState::Started); @@ -6366,7 +6589,8 @@ void profiler_ensure_started(PowerOfTwo32 aCapacity, double aInterval, } #if defined(MOZ_REPLACE_MALLOC) && defined(MOZ_PROFILER_MEMORY) - if (ActivePS::FeatureNativeAllocations(aLock)) { + if (ActivePS::FeatureNativeAllocations(aLock) && + ActivePS::ShouldInstallMemoryHooks(aLock)) { mozilla::profiler::disable_native_allocations(); } #endif diff --git a/tools/profiler/docs/code-overview.rst b/tools/profiler/docs/code-overview.rst index 3ca662e141..bb9db364de 100644 --- a/tools/profiler/docs/code-overview.rst +++ b/tools/profiler/docs/code-overview.rst @@ -2,7 +2,7 @@ Profiler Code Overview ###################### This is an overview of the code that implements the Profiler inside Firefox -with dome details around tricky subjects, or pointers to more detailed +with some details around tricky subjects, or pointers to more detailed documentation and/or source code. It assumes familiarity with Firefox development, including Mercurial (hg), mach, diff --git a/tools/profiler/docs/index.rst b/tools/profiler/docs/index.rst index 53920e7d2f..02eb9f6839 100644 --- a/tools/profiler/docs/index.rst +++ b/tools/profiler/docs/index.rst @@ -23,13 +23,13 @@ while the profiler.firefox.com interface is documented at `profiler.firefox.com/ buffer instrumenting-javascript instrumenting-rust + instrumenting-android markers-guide memory The following areas still need documentation: * LUL - * Instrumenting Java * Registering Threads * Samples and Stack Walking * Triggering Gecko Profiles in Automation diff --git a/tools/profiler/docs/instrumenting-android.rst b/tools/profiler/docs/instrumenting-android.rst new file mode 100644 index 0000000000..fdd96613b4 --- /dev/null +++ b/tools/profiler/docs/instrumenting-android.rst @@ -0,0 +1,102 @@ +Instrumenting Android +======================== + +There are multiple ways to use the profiler with Android. There is the "Java" +profiler feature (via about:profiling), which enables profiling for JVM code. +This is most likely turned on already for the preset if you are profiling an +Android device. + +In addition to sampling, markers can be created to specifically mark an instant +in time, or a duration. This can be helpful to make sense of a particular piece +of the front-end, or record events that normally wouldn't show up in samples. + +.. note:: + This guide explains Android markers in depth. To learn more about how to add a + marker in C++, JavaScript or Rust, please take a look at their documentation + in :doc:`markers-guide`, :doc:`instrumenting-javascript` or + :doc:`instrumenting-rust` respectively. + +Markers in the GeckoView Java codebase +************************************** + +If you are in the GeckoView codebase, then you should have access to ``GeckoRuntime``. +``GeckoRuntime`` has a ``getProfilerController`` method to get the ``ProfilerController``. +See the `ProfilerController Java file`_ (`javadoc`_) to find which methods you can use to +instrument your source code. + +Here's an example: + +.. code-block:: java + + // Simple marker + sGeckoRuntime.getProfilerController().addMarker("Marker Name"); + + // Simple marker with additional information + sGeckoRuntime.getProfilerController().addMarker("Marker Name", "info"); + + // Duration marker + Double startTime = sGeckoRuntime.getProfilerController().getProfilerTime(); + // ...some code you want to measure... + sGeckoRuntime.getProfilerController().addMarker("Marker Name", startTime); + + // Duration marker with additional information + sGeckoRuntime.getProfilerController().addMarker("Marker Name", startTime, "info"); + +There are various overloads of ``addMarker`` you can choose depending on your need. + +If you need to compute some information before adding it to a marker, it's +recommended to wrap that code with a `isProfilerActive` if check to make sure +that it's only executed while the profiler is active. Here's an example: + +.. code-block:: java + + ProfilerController profilerController = runtime.getProfilerController(); + if (profilerController.isProfilerActive()) { + // Compute the information you want to include. + String info = ... + sGeckoRuntime.getProfilerController().addMarker("Marker Name", info); + } + +Markers in the Fenix codebase +***************************** + +If you are in the Fenix codebase, then you should have access to the Android +components. The Android components includes a `Profiler interface here`_, with +its corresponding `implementation here`_. You should be able to do everything +you can do with the ``ProfilerController``. Here's an example on how to call them: + +.. code-block:: kotlin + + // Simple marker + components.core.engine.profiler?.addMarker("Marker Name"); + + // Simple marker with additional information + components.core.engine.profiler?.addMarker("Marker Name", "info"); + + // Duration marker + val startTime = components.core.engine.profiler?.getProfilerTime() + // ...some code you want to measure... + components.core.engine.profiler?.addMarker("Marker Name", startTime, "additional info") + + // Duration marker with additional information + components.core.engine.profiler?.addMarker("Marker Name", startTime, "info"); + +Similarly, there are various overloads of ``addMarker`` you can choose depending on your needs. + +Like for the GeckoView example above, if you need to compute some information +before adding it to a marker, it's recommended to wrap that code with a +`isProfilerActive` if check to make sure that it's only executed while the +profiler is active. Here's an example: + +.. code-block:: kotlin + + if (components.core.engine.profiler?.isProfilerActive() == true) { + // Compute the information you want to include. + var info = ... + components.core.engine.profiler?.addMarker("Marker Name", info) + } + +.. _ProfilerController Java file: https://searchfox.org/mozilla-central/source/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/ProfilerController.java +.. _javadoc: https://mozilla.github.io/geckoview/javadoc/mozilla-central/org/mozilla/geckoview/ProfilerController.html +.. _Profiler interface here: https://searchfox.org/mozilla-central/source/mobile/android/android-components/components/concept/base/src/main/java/mozilla/components/concept/base/profiler/Profiler.kt +.. _implementation here: https://searchfox.org/mozilla-central/source/mobile/android/android-components/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/profiler/Profiler.kt diff --git a/tools/profiler/docs/instrumenting-javascript.rst b/tools/profiler/docs/instrumenting-javascript.rst index 928d94781e..4ad5118256 100644 --- a/tools/profiler/docs/instrumenting-javascript.rst +++ b/tools/profiler/docs/instrumenting-javascript.rst @@ -11,8 +11,9 @@ or record events that normally wouldn't show up in samples. .. note:: This guide explains JavaScript markers in depth. To learn more about how to add a - marker in C++ or Rust, please take a look at their documentation - in :doc:`markers-guide` or :doc:`instrumenting-rust` respectively. + marker in C++, Rust or JVM please take a look at their documentation + in :doc:`markers-guide`, :doc:`instrumenting-rust` or :doc:`instrumenting-android` + respectively. Markers in Browser Chrome ************************* diff --git a/tools/profiler/docs/instrumenting-rust.rst b/tools/profiler/docs/instrumenting-rust.rst index c3e12f42dc..603c008b6c 100644 --- a/tools/profiler/docs/instrumenting-rust.rst +++ b/tools/profiler/docs/instrumenting-rust.rst @@ -124,8 +124,9 @@ and an optional payload of a specific type (containing arbitrary data relevant t .. note:: This guide explains Rust markers in depth. To learn more about how to add a - marker in C++ or JavaScript, please take a look at their documentation - in :doc:`markers-guide` or :doc:`instrumenting-javascript` respectively. + marker in C++, JavaScript or JVM, please take a look at their documentation + in :doc:`markers-guide` or :doc:`instrumenting-javascript`, + :doc:`instrumenting-android` respectively. Examples ^^^^^^^^ diff --git a/tools/profiler/docs/markers-guide.rst b/tools/profiler/docs/markers-guide.rst index ed18b35867..ac0e4430b9 100644 --- a/tools/profiler/docs/markers-guide.rst +++ b/tools/profiler/docs/markers-guide.rst @@ -9,8 +9,9 @@ and an optional payload of a specific type (containing arbitrary data relevant t .. note:: This guide explains C++ markers in depth. To learn more about how to add a - marker in JavaScript or Rust, please take a look at their documentation - in :doc:`instrumenting-javascript` or :doc:`instrumenting-rust` respectively. + marker in JavaScript, Rust or JVM, please take a look at their documentation + in :doc:`instrumenting-javascript`, :doc:`instrumenting-rust` or + :doc:`instrumenting-android` respectively. Example ------- diff --git a/tools/profiler/public/ProfilerControl.h b/tools/profiler/public/ProfilerControl.h index ac145fac00..53fec681ce 100644 --- a/tools/profiler/public/ProfilerControl.h +++ b/tools/profiler/public/ProfilerControl.h @@ -61,6 +61,9 @@ static constexpr mozilla::PowerOfTwo32 PROFILER_DEFAULT_ENTRIES = static constexpr mozilla::PowerOfTwo32 PROFILER_DEFAULT_STARTUP_ENTRIES = mozilla::baseprofiler::BASE_PROFILER_DEFAULT_STARTUP_ENTRIES; +static constexpr mozilla::PowerOfTwo32 PROFILER_DEFAULT_SIGHANDLE_ENTRIES = + mozilla::MakePowerOfTwo32<64 * 1024 * 1024>(); // 64M entries = 512MiB + # define PROFILER_DEFAULT_INTERVAL BASE_PROFILER_DEFAULT_INTERVAL # define PROFILER_MAX_INTERVAL BASE_PROFILER_MAX_INTERVAL diff --git a/tools/profiler/public/ProfilerCounts.h b/tools/profiler/public/ProfilerCounts.h index cebca81e2c..bdd2ddef9a 100644 --- a/tools/profiler/public/ProfilerCounts.h +++ b/tools/profiler/public/ProfilerCounts.h @@ -20,6 +20,7 @@ # include "mozilla/Assertions.h" # include "mozilla/Atomics.h" +# include "mozilla/DataMutex.h" class BaseProfilerCount; void profiler_add_sampled_counter(BaseProfilerCount* aCounter); @@ -188,13 +189,33 @@ class ProfilerCounterTotal final : public BaseProfilerCount { public: ProfilerCounterTotal(const char* aLabel, const char* aCategory, const char* aDescription) - : BaseProfilerCount(aLabel, &mCounter, &mNumber, aCategory, - aDescription) { + : BaseProfilerCount(aLabel, &mCounter, &mNumber, aCategory, aDescription), + mRegistered(false, "ProfilerCounterTotal::mRegistered") { // Assume we're in libxul + Register(); + } + + virtual ~ProfilerCounterTotal() { Unregister(); } + + void Register() { + auto registered = mRegistered.Lock(); + if (*registered) { + return; + } + + *registered = true; profiler_add_sampled_counter(this); } - virtual ~ProfilerCounterTotal() { profiler_remove_sampled_counter(this); } + void Unregister() { + auto registered = mRegistered.Lock(); + if (!*registered) { + return; + } + + *registered = false; + profiler_remove_sampled_counter(this); + } BaseProfilerCount& operator++() { Add(1); @@ -208,6 +229,9 @@ class ProfilerCounterTotal final : public BaseProfilerCount { ProfilerAtomicSigned mCounter; ProfilerAtomicUnsigned mNumber; + // Using OffTheBooksMutex here because we intentionally leak memory counters + // if they are initialized. + mozilla::DataMutexBase<bool, mozilla::OffTheBooksMutex> mRegistered; }; // Defines a counter that is sampled on each profiler tick, with a running diff --git a/tools/profiler/public/ProfilerState.h b/tools/profiler/public/ProfilerState.h index 40e1517c91..aad74862b3 100644 --- a/tools/profiler/public/ProfilerState.h +++ b/tools/profiler/public/ProfilerState.h @@ -118,7 +118,10 @@ "every CPU core for every profiler sample.") \ \ MACRO(23, "bandwidth", Bandwidth, \ - "Record the network bandwidth used for every profiler sample.") + "Record the network bandwidth used for every profiler sample.") \ + MACRO( \ + 24, "memory", Memory, \ + "Track the memory allocations and deallocations per process over time.") // *** Synchronize with lists in BaseProfilerState.h and geckoProfiler.json *** struct ProfilerFeature { @@ -138,6 +141,12 @@ struct ProfilerFeature { PROFILER_FOR_EACH_FEATURE(DECLARE) #undef DECLARE + + [[nodiscard]] static constexpr bool ShouldInstallMemoryHooks( + uint32_t aFeatures) { + return ProfilerFeature::HasMemory(aFeatures) || + ProfilerFeature::HasNativeAllocations(aFeatures); + } }; // clang-format off diff --git a/tools/profiler/tests/gtest/moz.build b/tools/profiler/tests/gtest/moz.build index e1c3994621..ea5ca1f228 100644 --- a/tools/profiler/tests/gtest/moz.build +++ b/tools/profiler/tests/gtest/moz.build @@ -30,7 +30,8 @@ LOCAL_INCLUDES += [ "/tools/profiler/lul", ] -if CONFIG["OS_TARGET"] != "Android": +# Bug 1885381 - Hangs/timeouts under TSAN +if CONFIG["OS_TARGET"] != "Android" and not CONFIG["MOZ_TSAN"]: UNIFIED_SOURCES += [ "GeckoProfiler.cpp", "ThreadProfileTest.cpp", diff --git a/tools/profiler/tests/xpcshell/test_feature_posix_signals.js b/tools/profiler/tests/xpcshell/test_feature_posix_signals.js index 28fbf890e8..d4b77d6550 100644 --- a/tools/profiler/tests/xpcshell/test_feature_posix_signals.js +++ b/tools/profiler/tests/xpcshell/test_feature_posix_signals.js @@ -5,7 +5,7 @@ ChromeUtils.defineESModuleGetters(this, { Downloads: "resource://gre/modules/Downloads.sys.mjs", FileUtils: "resource://gre/modules/FileUtils.sys.mjs", - BrowserTestUtils: "resource://testing-common/BrowserTestUtils.sys.mjs", + TestUtils: "resource://testing-common/TestUtils.sys.mjs", }); const { ctypes } = ChromeUtils.importESModule( @@ -68,23 +68,6 @@ function raiseSignal(pid, sig) { return { ok: true }; } -// We would like to use the following to wait for a stop signal to actually be -// handled: -// await Services.profiler.waitOnePeriodicSampling(); -// However, as we are trying to shut down the profiler using the sampler -// thread, this can cause complications between the callback waiting for the -// sampling to be over, and the sampler thread actually finishing. -// Instead, we use the BrowserTestUtils.waitForCondition to wait until the -// profiler is no longer active. -async function waitUntilProfilerStopped(interval = 1000, maxTries = 100) { - await BrowserTestUtils.waitForCondition( - () => !Services.profiler.IsActive(), - "the profiler should be inactive", - interval, - maxTries - ); -} - async function cleanupAfterTest() { // We need to cleanup written profiles after a test // Get the system downloads directory, and use it to build a profile file @@ -112,10 +95,46 @@ async function cleanupAfterTest() { // may cause a mismatch if we test on on, say, a gnu hurd kernel, or on a // linux kernel running on sparc, but the feature will not break - only // the testing. -// const SIGUSR1 = Services.appinfo.OS === "Darwin" ? 30 : 10; +const SIGUSR1 = Services.appinfo.OS === "Darwin" ? 30 : 10; const SIGUSR2 = Services.appinfo.OS === "Darwin" ? 31 : 12; add_task(async () => { + info("Test that starting the profiler with a posix signal works."); + + Assert.ok( + !Services.profiler.IsActive(), + "The profiler should not begin the test active." + ); + + // Get the process ID + let pid = Services.appinfo.processID; + + // Set up an observer to watch for the profiler starting + let startPromise = TestUtils.topicObserved("profiler-started"); + + // Try and start the profiler using a signal. + let result = raiseSignal(pid, SIGUSR1); + Assert.ok(result, "Raising a signal should succeed"); + + // Wait for the profiler to stop + Assert.ok(await startPromise, "The profiler should start"); + + // Wait until the profiler is active + Assert.ok(Services.profiler.IsActive(), "The profiler should now be active."); + + // Let the profiler sample at least once + await Services.profiler.waitOnePeriodicSampling(); + info("Waiting a periodic sampling completed"); + + // Stop the profiler + await Services.profiler.StopProfiler(); + Assert.ok( + !Services.profiler.IsActive(), + "The profiler should now be inactive." + ); +}); + +add_task(async () => { info("Test that stopping the profiler with a posix signal works."); registerCleanupFunction(cleanupAfterTest); @@ -136,13 +155,20 @@ add_task(async () => { // Get the process ID let pid = Services.appinfo.processID; + // Set up an observer to watch for the profiler stopping + let stopPromise = TestUtils.topicObserved("profiler-stopped"); + // Try and stop the profiler using a signal. let result = raiseSignal(pid, SIGUSR2); Assert.ok(result, "Raising a SIGUSR2 signal should succeed."); - await waitUntilProfilerStopped(); + // Wait for the profiler to stop + Assert.ok(await stopPromise, "The profiler should stop"); - do_test_finished(); + Assert.ok( + !Services.profiler.IsActive(), + "The profiler should now be inactive." + ); }); add_task(async () => { @@ -174,21 +200,24 @@ add_task(async () => { await Services.profiler.StartProfiler(entries, interval, threads, features); Assert.ok(Services.profiler.IsActive(), "The profiler should now be active."); + // Set up an observer to watch for the profiler stopping + let stopPromise = TestUtils.topicObserved("profiler-stopped"); + // Try and stop the profiler using a signal. let result = raiseSignal(pid, SIGUSR2); Assert.ok(result, "Raising a SIGUSR2 signal should succeed."); - // Wait for the file to exist - await BrowserTestUtils.waitForCondition( - async () => await IOUtils.exists(profile.path), - "Waiting for a profile file to be written to disk." + // Wait for the profiler to stop + Assert.ok(await stopPromise, "The profiler should stop"); + + // Now that it's stopped, make sure that we have a profile file + Assert.ok( + await IOUtils.exists(profile.path), + "A profile file should be written to disk." ); - await waitUntilProfilerStopped(); Assert.ok( !Services.profiler.IsActive(), "The profiler should now be inactive." ); - - do_test_finished(); }); diff --git a/tools/profiler/tests/xpcshell/xpcshell.toml b/tools/profiler/tests/xpcshell/xpcshell.toml index 2cde39d09f..8e58e1ff08 100644 --- a/tools/profiler/tests/xpcshell/xpcshell.toml +++ b/tools/profiler/tests/xpcshell/xpcshell.toml @@ -49,8 +49,10 @@ skip-if = [ ["test_feature_posix_signals.js"] skip-if = [ - "ccov", - "os == 'win'", + "tsan", # We have intermittent timeout issues in TSan, see Bug 1889828 + "ccov", # The signals for the profiler conflict with the ccov signals + "os == 'win'", # Not yet supported on windows + "os == 'android'", # Not yet supported on android ] # Native stackwalking is somewhat unreliable depending on the platform. |