[Sampling profiler] Create Unwinders on profiler thread

Moves creation of Unwinders from the profiled thread to the profiler
thread on Android, Windows, and Mac. This initialization is expensive
on Android -- 50+ ms -- so it's important to not block the profiled
thread on performing it. Mac does some work in its unwinder
construction so might see a small benefit as well.

This is accomplished by passing a factory function for creating
the unwinders into StackSamplingProfiler rather than the unwinders
themselves. The naming of the classes that create the individual
unwinders in ThreadProfiler is updated to call them 'creators',
to minimize confusion over multiple levels of factories.

Also updates class-level documentation on how to use the profiler.

Bug: 1129082
Change-Id: I791538c72dce5c0d04c4f30e2f287fef96da1db6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414773
Commit-Queue: Mike Wittman <wittman@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: ssid <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809202}

base/profiler/stack_sampler.h

@@ -27,6 +27,10 @@ class StackSamplerTestDelegate;
 // for a given thread.
 class BASE_EXPORT StackSampler {
  public:
+  // Factory for generating a set of Unwinders for use by the profiler.
+  using UnwindersFactory =
+      OnceCallback<std::vector<std::unique_ptr<Unwinder>>()>;
+
   virtual ~StackSampler();
 
   // Creates a stack sampler that records samples for thread with
@@ -37,7 +41,7 @@ class BASE_EXPORT StackSampler {
   static std::unique_ptr<StackSampler> Create(
       SamplingProfilerThreadToken thread_token,
       ModuleCache* module_cache,
-      std::vector<std::unique_ptr<Unwinder>> core_unwinders,
+      UnwindersFactory core_unwinders_factory,
       RepeatingClosure record_sample_callback,
       StackSamplerTestDelegate* test_delegate);
 
@@ -51,6 +55,9 @@ class BASE_EXPORT StackSampler {
   // The following functions are all called on the SamplingThread (not the
   // thread being sampled).
 
+  // Performs post-construction initialization on the SamplingThread.
+  virtual void Initialize() {}
+
   // Adds an auxiliary unwinder to handle additional, non-native-code unwind
   // scenarios. Unwinders must be inserted in increasing priority, following
   // |unwinders| provided in Create(), to guide unwind attempts.

base/profiler/stack_sampler_android.cc

@@ -18,13 +18,13 @@ namespace base {
 std::unique_ptr<StackSampler> StackSampler::Create(
     SamplingProfilerThreadToken thread_token,
     ModuleCache* module_cache,
-    std::vector<std::unique_ptr<Unwinder>> core_unwinders,
+    UnwindersFactory core_unwinders_factory,
     RepeatingClosure record_sample_callback,
     StackSamplerTestDelegate* test_delegate) {
   return std::make_unique<StackSamplerImpl>(
       std::make_unique<StackCopierSignal>(
           std::make_unique<ThreadDelegatePosix>(thread_token)),
-      std::move(core_unwinders), module_cache,
+      std::move(core_unwinders_factory), module_cache,
       std::move(record_sample_callback), test_delegate);
 }
 

base/profiler/stack_sampler_impl.cc

@@ -64,28 +64,36 @@ class StackCopierDelegate : public StackCopier::Delegate {
 
 }  // namespace
 
-// |core_unwinders| is iterated backward since |core_unwinders| is passed in
-// increasing priority order while |unwinders_| is stored in decreasing priority
-// order.
-StackSamplerImpl::StackSamplerImpl(
-    std::unique_ptr<StackCopier> stack_copier,
-    std::vector<std::unique_ptr<Unwinder>> core_unwinders,
-    ModuleCache* module_cache,
-    RepeatingClosure record_sample_callback,
-    StackSamplerTestDelegate* test_delegate)
+StackSamplerImpl::StackSamplerImpl(std::unique_ptr<StackCopier> stack_copier,
+                                   UnwindersFactory core_unwinders_factory,
+                                   ModuleCache* module_cache,
+                                   RepeatingClosure record_sample_callback,
+                                   StackSamplerTestDelegate* test_delegate)
     : stack_copier_(std::move(stack_copier)),
-      unwinders_(std::make_move_iterator(core_unwinders.rbegin()),
-                 std::make_move_iterator(core_unwinders.rend())),
+      unwinders_factory_(std::move(core_unwinders_factory)),
       module_cache_(module_cache),
       record_sample_callback_(std::move(record_sample_callback)),
       test_delegate_(test_delegate) {
-  DCHECK(!unwinders_.empty());
-  for (const auto& unwinder : unwinders_)
-    unwinder->AddInitialModules(module_cache_);
+  DCHECK(unwinders_factory_);
 }
 
 StackSamplerImpl::~StackSamplerImpl() = default;
 
+void StackSamplerImpl::Initialize() {
+  std::vector<std::unique_ptr<Unwinder>> unwinders =
+      std::move(unwinders_factory_).Run();
+
+  // |unwinders| is iterated backward since |unwinders_factory_| generates
+  // unwinders in increasing priority order. |unwinders_| is stored in
+  // decreasing priority order for ease of use within the class.
+  unwinders_.insert(unwinders_.end(),
+                    std::make_move_iterator(unwinders.rbegin()),
+                    std::make_move_iterator(unwinders.rend()));
+
+  for (const auto& unwinder : unwinders_)
+    unwinder->AddInitialModules(module_cache_);
+}
+
 void StackSamplerImpl::AddAuxUnwinder(std::unique_ptr<Unwinder> unwinder) {
   unwinder->AddInitialModules(module_cache_);
   unwinders_.push_front(std::move(unwinder));

base/profiler/stack_sampler_impl.h

@@ -24,7 +24,7 @@ class Unwinder;
 class BASE_EXPORT StackSamplerImpl : public StackSampler {
  public:
   StackSamplerImpl(std::unique_ptr<StackCopier> stack_copier,
-                   std::vector<std::unique_ptr<Unwinder>> core_unwinders,
+                   UnwindersFactory core_unwinders_factory,
                    ModuleCache* module_cache,
                    RepeatingClosure record_sample_callback = RepeatingClosure(),
                    StackSamplerTestDelegate* test_delegate = nullptr);
@@ -34,6 +34,7 @@ class BASE_EXPORT StackSamplerImpl : public StackSampler {
   StackSamplerImpl& operator=(const StackSamplerImpl&) = delete;
 
   // StackSampler:
+  void Initialize() override;
   void AddAuxUnwinder(std::unique_ptr<Unwinder> unwinder) override;
   void RecordStackFrames(StackBuffer* stack_buffer,
                          ProfileBuilder* profile_builder) override;
@@ -53,8 +54,11 @@ class BASE_EXPORT StackSamplerImpl : public StackSampler {
       const base::circular_deque<std::unique_ptr<Unwinder>>& unwinders);
 
   const std::unique_ptr<StackCopier> stack_copier_;
-  // Store all unwinder in decreasing priority order.
+  UnwindersFactory unwinders_factory_;
+
+  // Unwinders are stored in decreasing priority order.
   base::circular_deque<std::unique_ptr<Unwinder>> unwinders_;
+
   ModuleCache* const module_cache_;
   const RepeatingClosure record_sample_callback_;
   StackSamplerTestDelegate* const test_delegate_;

base/profiler/stack_sampler_impl_unittest.cc

@@ -9,6 +9,7 @@
 #include <numeric>
 #include <utility>
 
+#include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #include "base/profiler/module_cache.h"
 #include "base/profiler/profile_builder.h"
@@ -268,11 +269,15 @@ class FakeTestUnwinder : public Unwinder {
   std::vector<Result> results_;
 };
 
-std::vector<std::unique_ptr<Unwinder>> MakeUnwinderVector(
+StackSampler::UnwindersFactory MakeUnwindersFactory(
     std::unique_ptr<Unwinder> unwinder) {
-  std::vector<std::unique_ptr<Unwinder>> unwinders;
-  unwinders.push_back(std::move(unwinder));
-  return unwinders;
+  return BindOnce(
+      [](std::unique_ptr<Unwinder> unwinder) {
+        std::vector<std::unique_ptr<Unwinder>> unwinders;
+        unwinders.push_back(std::move(unwinder));
+        return unwinders;
+      },
+      std::move(unwinder));
 }
 
 base::circular_deque<std::unique_ptr<Unwinder>> MakeUnwinderCircularDeque(
@@ -301,10 +306,12 @@ TEST(StackSamplerImplTest, MAYBE_CopyStack) {
   std::vector<uintptr_t> stack_copy;
   StackSamplerImpl stack_sampler_impl(
       std::make_unique<TestStackCopier>(stack),
-      MakeUnwinderVector(
+      MakeUnwindersFactory(
           std::make_unique<TestUnwinder>(stack.size(), &stack_copy)),
       &module_cache);
 
+  stack_sampler_impl.Initialize();
+
   std::unique_ptr<StackBuffer> stack_buffer =
       std::make_unique<StackBuffer>(stack.size() * sizeof(uintptr_t));
   TestProfileBuilder profile_builder(&module_cache);
@@ -321,10 +328,12 @@ TEST(StackSamplerImplTest, CopyStackTimestamp) {
   TimeTicks timestamp = TimeTicks::UnixEpoch();
   StackSamplerImpl stack_sampler_impl(
       std::make_unique<TestStackCopier>(stack, timestamp),
-      MakeUnwinderVector(
+      MakeUnwindersFactory(
           std::make_unique<TestUnwinder>(stack.size(), &stack_copy)),
       &module_cache);
 
+  stack_sampler_impl.Initialize();
+
   std::unique_ptr<StackBuffer> stack_buffer =
       std::make_unique<StackBuffer>(stack.size() * sizeof(uintptr_t));
   TestProfileBuilder profile_builder(&module_cache);
@@ -341,7 +350,9 @@ TEST(StackSamplerImplTest, UnwinderInvokedWhileRecordingStackFrames) {
   TestProfileBuilder profile_builder(&module_cache);
   StackSamplerImpl stack_sampler_impl(
       std::make_unique<DelegateInvokingStackCopier>(),
-      MakeUnwinderVector(std::move(owned_unwinder)), &module_cache);
+      MakeUnwindersFactory(std::move(owned_unwinder)), &module_cache);
+
+  stack_sampler_impl.Initialize();
 
   stack_sampler_impl.RecordStackFrames(stack_buffer.get(), &profile_builder);
 
@@ -355,9 +366,11 @@ TEST(StackSamplerImplTest, AuxUnwinderInvokedWhileRecordingStackFrames) {
   TestProfileBuilder profile_builder(&module_cache);
   StackSamplerImpl stack_sampler_impl(
       std::make_unique<DelegateInvokingStackCopier>(),
-      MakeUnwinderVector(std::make_unique<CallRecordingUnwinder>()),
+      MakeUnwindersFactory(std::make_unique<CallRecordingUnwinder>()),
       &module_cache);
 
+  stack_sampler_impl.Initialize();
+
   auto owned_aux_unwinder = std::make_unique<CallRecordingUnwinder>();
   CallRecordingUnwinder* aux_unwinder = owned_aux_unwinder.get();
   stack_sampler_impl.AddAuxUnwinder(std::move(owned_aux_unwinder));

base/profiler/stack_sampler_ios.cc

@@ -13,7 +13,7 @@ namespace base {
 std::unique_ptr<StackSampler> StackSampler::Create(
     SamplingProfilerThreadToken thread_token,
     ModuleCache* module_cache,
-    std::vector<std::unique_ptr<Unwinder>> core_unwinders,
+    UnwindersFactory core_unwinders_factory,
     RepeatingClosure record_sample_callback,
     StackSamplerTestDelegate* test_delegate) {
   return nullptr;

base/profiler/stack_sampler_mac.cc

@@ -4,6 +4,7 @@
 
 #include "base/profiler/stack_sampler.h"
 
+#include "base/bind.h"
 #include "base/check.h"
 #include "base/profiler/native_unwinder_mac.h"
 #include "base/profiler/stack_copier_suspend.h"
@@ -12,19 +13,29 @@
 
 namespace base {
 
+namespace {
+
+std::vector<std::unique_ptr<Unwinder>> CreateUnwinders(
+    ModuleCache* module_cache) {
+  std::vector<std::unique_ptr<Unwinder>> unwinders;
+  unwinders.push_back(std::make_unique<NativeUnwinderMac>(module_cache));
+  return unwinders;
+}
+
+}  // namespace
+
 // static
 std::unique_ptr<StackSampler> StackSampler::Create(
     SamplingProfilerThreadToken thread_token,
     ModuleCache* module_cache,
-    std::vector<std::unique_ptr<Unwinder>> core_unwinders,
+    UnwindersFactory core_unwinders_factory,
     RepeatingClosure record_sample_callback,
     StackSamplerTestDelegate* test_delegate) {
-  DCHECK(core_unwinders.empty());
-  core_unwinders.push_back(std::make_unique<NativeUnwinderMac>(module_cache));
+  DCHECK(!core_unwinders_factory);
   return std::make_unique<StackSamplerImpl>(
       std::make_unique<StackCopierSuspend>(
           std::make_unique<SuspendableThreadDelegateMac>(thread_token)),
-      std::move(core_unwinders), module_cache,
+      BindOnce(&CreateUnwinders, Unretained(module_cache)), module_cache,
       std::move(record_sample_callback), test_delegate);
 }
 

base/profiler/stack_sampler_posix.cc

@@ -14,7 +14,7 @@ namespace base {
 std::unique_ptr<StackSampler> StackSampler::Create(
     SamplingProfilerThreadToken thread_token,
     ModuleCache* module_cache,
-    std::vector<std::unique_ptr<Unwinder>> core_unwinders,
+    UnwindersFactory core_unwinders_factory,
     RepeatingClosure record_sample_callback,
     StackSamplerTestDelegate* test_delegate) {
   return nullptr;

base/profiler/stack_sampler_win.cc

@@ -4,6 +4,7 @@
 
 #include "base/profiler/stack_sampler.h"
 
+#include "base/bind.h"
 #include "base/check.h"
 #include "base/profiler/native_unwinder_win.h"
 #include "base/profiler/stack_copier_suspend.h"
@@ -17,16 +18,20 @@ namespace base {
 std::unique_ptr<StackSampler> StackSampler::Create(
     SamplingProfilerThreadToken thread_token,
     ModuleCache* module_cache,
-    std::vector<std::unique_ptr<Unwinder>> core_unwinders,
+    UnwindersFactory core_unwinders_factory,
     RepeatingClosure record_sample_callback,
     StackSamplerTestDelegate* test_delegate) {
-  DCHECK(core_unwinders.empty());
+  DCHECK(!core_unwinders_factory);
 #if defined(ARCH_CPU_X86_64) || defined(ARCH_CPU_ARM64)
-  core_unwinders.push_back(std::make_unique<NativeUnwinderWin>());
+  const auto create_unwinders = []() {
+    std::vector<std::unique_ptr<Unwinder>> unwinders;
+    unwinders.push_back(std::make_unique<NativeUnwinderWin>());
+    return unwinders;
+  };
   return std::make_unique<StackSamplerImpl>(
       std::make_unique<StackCopierSuspend>(
           std::make_unique<SuspendableThreadDelegateWin>(thread_token)),
-      std::move(core_unwinders), module_cache,
+      BindOnce(create_unwinders), module_cache,
       std::move(record_sample_callback), test_delegate);
 #else
   return nullptr;

base/profiler/stack_sampling_profiler.cc

@@ -579,6 +579,8 @@ void StackSamplingProfiler::SamplingThread::AddCollectionTask(
   const int collection_id = collection->collection_id;
   const TimeDelta initial_delay = collection->params.initial_delay;
 
+  collection->sampler->Initialize();
+
   active_collections_.insert(
       std::make_pair(collection_id, std::move(collection)));
 
@@ -773,13 +775,14 @@ StackSamplingProfiler::StackSamplingProfiler(
     SamplingProfilerThreadToken thread_token,
     const SamplingParams& params,
     std::unique_ptr<ProfileBuilder> profile_builder,
-    std::vector<std::unique_ptr<Unwinder>> unwinders,
+    UnwindersFactory core_unwinders_factory,
     RepeatingClosure record_sample_callback,
     StackSamplerTestDelegate* test_delegate)
     : StackSamplingProfiler(params, std::move(profile_builder), nullptr) {
-  sampler_ = StackSampler::Create(
-      thread_token, profile_builder_->GetModuleCache(), std::move(unwinders),
-      std::move(record_sample_callback), test_delegate);
+  sampler_ =
+      StackSampler::Create(thread_token, profile_builder_->GetModuleCache(),
+                           std::move(core_unwinders_factory),
+                           std::move(record_sample_callback), test_delegate);
 }
 
 StackSamplingProfiler::StackSamplingProfiler(

base/profiler/stack_sampling_profiler.h

@@ -35,21 +35,29 @@ class StackSamplerTestDelegate;
 //   // Create and customize params as desired.
 //   base::StackStackSamplingProfiler::SamplingParams params;
 //
-//   // To process the profiles, use a custom ProfileBuilder subclass:
-//   class SubProfileBuilder : public base::ProfileBuilder {...}
+//   // Create a ProfileBuilder subclass to process the profiles.
+//   class ProfileBuilder : public base::ProfileBuilder {...}
 //
 //   // Then create the profiler:
-//   base::StackSamplingProfiler profiler(base::PlatformThread::CurrentId(),
-//       params, std::make_unique<SubProfileBuilder>(...));
+//   base::StackSamplingProfiler profiler(
+//       GetSamplingProfilerCurrentThreadToken(),
+//       params,
+//       std::make_unique<ProfileBuilder>(...));
 //
-//   // On Android the |sampler| is not implemented in base. So, client can pass
-//   // in |sampler| to use while profiling.
-//   base::StackSamplingProfiler profiler(base::PlatformThread::CurrentId(),
-//       params, std::make_unique<SubProfileBuilder>(...), <optional> sampler);
+//   // On Android the caller also must provide a factory function for creating
+//   // its core stack unwinders. See the ThreadProfiler implementation for an
+//   // example of how to do this.
+//   base::StackSamplingProfiler profiler(
+//       GetSamplingProfilerCurrentThreadToken(),
+//       params,
+//       std::make_unique<ProfileBuilder>(...),
+//       core_unwinders_factory);
 //
 //   // Then start the profiling.
 //   profiler.Start();
+//
 //   // ... work being done on the target thread here ...
+//
 //   // Optionally stop collection before complete per params.
 //   profiler.Stop();
 //
@@ -62,6 +70,13 @@ class StackSamplerTestDelegate;
 // by the profiler.
 class BASE_EXPORT StackSamplingProfiler {
  public:
+  // Factory for generating a set of Unwinders for use by the profiler. More
+  // general unwinders should appear before more specific unwinders in the
+  // generated vector, e.g. a system unwinder should appear before a Chrome
+  // unwinder. The callback will be invoked on the profiler thread.
+  using UnwindersFactory =
+      OnceCallback<std::vector<std::unique_ptr<Unwinder>>()>;
+
   // Represents parameters that configure the sampling.
   struct BASE_EXPORT SamplingParams {
     // Time to delay before first samples are taken.
@@ -79,12 +94,12 @@ class BASE_EXPORT StackSamplingProfiler {
   // configuration.
   static bool IsSupported();
 
-  // Creates a profiler for the specified thread. |unwinders| is required on
-  // Android since the unwinder is provided outside StackSamplingProfiler, but
-  // must be empty on other platforms. When attempting to unwind, the relative
-  // priority of unwinders is the inverse of the order in |unwinders|.
-  // |record_sample_callback| is made for each sample right before recording the
-  // stack sample. An optional |test_delegate| can be supplied by tests.
+  // Creates a profiler for the the thread associated with |thread_token|,
+  // generated by GetSamplingProfilerCurrentThreadToken().
+  // |core_unwinders_factory| is required on Android since the unwinders are
+  // provided outside StackSamplingProfiler, but must be null other platforms.
+  // |record_sample_callback| is called for each sample right before recording
+  // the stack sample. An optional |test_delegate| can be supplied by tests.
   //
   // The caller must ensure that this object gets destroyed before the thread
   // exits.
@@ -92,7 +107,7 @@ class BASE_EXPORT StackSamplingProfiler {
       SamplingProfilerThreadToken thread_token,
       const SamplingParams& params,
       std::unique_ptr<ProfileBuilder> profile_builder,
-      std::vector<std::unique_ptr<Unwinder>> core_unwinders = {},
+      UnwindersFactory core_unwinders_factory = UnwindersFactory(),
       RepeatingClosure record_sample_callback = RepeatingClosure(),
       StackSamplerTestDelegate* test_delegate = nullptr);
 

base/profiler/stack_sampling_profiler_test_util.cc

@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "base/bind.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/location.h"
@@ -317,7 +318,7 @@ std::vector<Frame> SampleScenario(UnwindScenario* scenario,
                       sample = std::move(result_sample);
                       sampling_thread_completed.Signal();
                     })),
-                CreateCoreUnwindersForTesting(module_cache));
+                CreateCoreUnwindersFactoryForTesting(module_cache));
             if (aux_unwinder_factory)
               profiler.AddAuxUnwinder(std::move(aux_unwinder_factory).Run());
             profiler.Start();
@@ -415,7 +416,7 @@ uintptr_t GetAddressInOtherLibrary(NativeLibrary library) {
   return address;
 }
 
-std::vector<std::unique_ptr<Unwinder>> CreateCoreUnwindersForTesting(
+StackSamplingProfiler::UnwindersFactory CreateCoreUnwindersFactoryForTesting(
     ModuleCache* module_cache) {
 #if defined(OS_ANDROID) && BUILDFLAG(ENABLE_ARM_CFI_TABLE)
   std::vector<std::unique_ptr<Unwinder>> unwinders;
@@ -423,9 +424,13 @@ std::vector<std::unique_ptr<Unwinder>> CreateCoreUnwindersForTesting(
       reinterpret_cast<uintptr_t>(&__executable_start)));
   unwinders.push_back(CreateChromeUnwinderAndroidForTesting(
       reinterpret_cast<uintptr_t>(&__executable_start)));
-  return unwinders;
+  return BindOnce(
+      [](std::vector<std::unique_ptr<Unwinder>> unwinders) {
+        return unwinders;
+      },
+      std::move(unwinders));
 #else
-  return {};
+  return StackSamplingProfiler::UnwindersFactory();
 #endif
 }
 

base/profiler/stack_sampling_profiler_test_util.h

@@ -12,6 +12,7 @@
 #include "base/native_library.h"
 #include "base/profiler/frame.h"
 #include "base/profiler/sampling_profiler_thread_token.h"
+#include "base/profiler/stack_sampling_profiler.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/platform_thread.h"
 
@@ -143,7 +144,7 @@ uintptr_t GetAddressInOtherLibrary(NativeLibrary library);
 // Creates a list of core unwinders required for StackSamplingProfilerTest.
 // This is useful notably on Android, which requires ChromeUnwinderAndroid in
 // addition to the native one.
-std::vector<std::unique_ptr<Unwinder>> CreateCoreUnwindersForTesting(
+StackSamplingProfiler::UnwindersFactory CreateCoreUnwindersFactoryForTesting(
     ModuleCache* module_cache);
 
 }  // namespace base

base/profiler/stack_sampling_profiler_unittest.cc

@@ -216,7 +216,7 @@ struct TestProfilerInfo {
                        profile = std::move(result_profile);
                        completed.Signal();
                      })),
-                 CreateCoreUnwindersForTesting(module_cache),
+                 CreateCoreUnwindersFactoryForTesting(module_cache),
                  RepeatingClosure(),
                  delegate) {}
 
@@ -351,7 +351,7 @@ void TestLibraryUnload(bool wait_until_unloaded, ModuleCache* module_cache) {
                 profile = std::move(result_profile);
                 sampling_thread_completed.Signal();
               })),
-      CreateCoreUnwindersForTesting(module_cache), RepeatingClosure(),
+      CreateCoreUnwindersFactoryForTesting(module_cache), RepeatingClosure(),
       &test_delegate);
 
   profiler.Start();
@@ -593,7 +593,7 @@ PROFILER_TEST_F(StackSamplingProfilerTest, StopWithoutStarting) {
                       profile = std::move(result_profile);
                       sampling_completed.Signal();
                     })),
-            CreateCoreUnwindersForTesting(module_cache()));
+            CreateCoreUnwindersFactoryForTesting(module_cache()));
 
         profiler.Stop();  // Constructed but never started.
         EXPECT_FALSE(sampling_completed.IsSignaled());
@@ -814,7 +814,7 @@ PROFILER_TEST_F(StackSamplingProfilerTest, DestroyProfilerWhileProfiling) {
         }));
     profiler.reset(new StackSamplingProfiler(
         target_thread_token, params, std::move(profile_builder),
-        CreateCoreUnwindersForTesting(module_cache())));
+        CreateCoreUnwindersFactoryForTesting(module_cache())));
     profiler->Start();
     profiler.reset();
 
@@ -1166,7 +1166,7 @@ PROFILER_TEST_F(StackSamplingProfilerTest, MultipleSampledThreads) {
                 profile1 = std::move(result_profile);
                 sampling_thread_completed1.Signal();
               })),
-      CreateCoreUnwindersForTesting(module_cache()));
+      CreateCoreUnwindersFactoryForTesting(module_cache()));
 
   WaitableEvent sampling_thread_completed2(
       WaitableEvent::ResetPolicy::MANUAL,
@@ -1180,7 +1180,7 @@ PROFILER_TEST_F(StackSamplingProfilerTest, MultipleSampledThreads) {
                 profile2 = std::move(result_profile);
                 sampling_thread_completed2.Signal();
               })),
-      CreateCoreUnwindersForTesting(module_cache()));
+      CreateCoreUnwindersFactoryForTesting(module_cache()));
 
   // Finally the real work.
   profiler1.Start();
@@ -1216,7 +1216,7 @@ class ProfilerThread : public SimpleThread {
                         profile_ = std::move(result_profile);
                         completed_.Signal();
                       })),
-                  CreateCoreUnwindersForTesting(module_cache)) {}
+                  CreateCoreUnwindersFactoryForTesting(module_cache)) {}
   void Run() override {
     run_.Wait();
     profiler_.Start();
@@ -1302,7 +1302,7 @@ PROFILER_TEST_F(StackSamplingProfilerTest, AddAuxUnwinder_BeforeStart) {
                       profile = std::move(result_profile);
                       sampling_thread_completed.Signal();
                     })),
-                CreateCoreUnwindersForTesting(module_cache()));
+                CreateCoreUnwindersFactoryForTesting(module_cache()));
             profiler.AddAuxUnwinder(
                 std::make_unique<TestAuxUnwinder>(Frame(23, nullptr)));
             profiler.Start();
@@ -1343,7 +1343,7 @@ PROFILER_TEST_F(StackSamplingProfilerTest, AddAuxUnwinder_AfterStart) {
                       profile = std::move(result_profile);
                       sampling_thread_completed.Signal();
                     })),
-                CreateCoreUnwindersForTesting(module_cache()));
+                CreateCoreUnwindersFactoryForTesting(module_cache()));
             profiler.Start();
             profiler.AddAuxUnwinder(
                 std::make_unique<TestAuxUnwinder>(Frame(23, nullptr)));
@@ -1384,7 +1384,7 @@ PROFILER_TEST_F(StackSamplingProfilerTest, AddAuxUnwinder_AfterStop) {
                       profile = std::move(result_profile);
                       sampling_thread_completed.Signal();
                     })),
-                CreateCoreUnwindersForTesting(module_cache()));
+                CreateCoreUnwindersFactoryForTesting(module_cache()));
             profiler.Start();
             profiler.Stop();
             profiler.AddAuxUnwinder(
@@ -1458,7 +1458,7 @@ PROFILER_TEST_F(StackSamplingProfilerTest,
                     BindLambdaForTesting([&profile](Profile result_profile) {
                       profile = std::move(result_profile);
                     })),
-                CreateCoreUnwindersForTesting(module_cache()),
+                CreateCoreUnwindersFactoryForTesting(module_cache()),
                 RepeatingClosure(), &post_sample_invoker);
             profiler.Start();
             // Wait for 5 samples to be collected.

chrome/common/profiler/thread_profiler.cc

@@ -31,6 +31,9 @@
 #include "services/service_manager/embedder/switches.h"
 
 #if defined(OS_ANDROID) && BUILDFLAG(ENABLE_ARM_CFI_TABLE)
+#include <sys/types.h>
+#include <unistd.h>
+
 #include "base/android/apk_assets.h"
 #include "base/files/memory_mapped_file.h"
 #include "base/profiler/arm_cfi_table.h"
@@ -101,67 +104,92 @@ bool IsCurrentProcessBackgrounded() {
 #endif  // defined(OS_MAC)
 }
 
-const base::RepeatingCallback<std::vector<std::unique_ptr<base::Unwinder>>()>&
-GetCoreUnwindersFactory() {
-  const auto create_unwinders_factory = []() {
 #if defined(OS_ANDROID) && BUILDFLAG(ENABLE_ARM_CFI_TABLE)
-    static constexpr char kCfiFileName[] = "assets/unwind_cfi_32";
-
-    // The module is loadable if the profiler is enabled for the current
-    // process.
-    CHECK(ThreadProfilerConfiguration::Get()
-              ->IsProfilerEnabledForCurrentProcess());
-
-    class UnwindersFactory {
-     public:
-      UnwindersFactory()
-          : module_(stack_unwinder::Module::Load()),
-            memory_regions_map_(module_->CreateMemoryRegionsMap()) {
-        base::MemoryMappedFile::Region cfi_region;
-        int fd = base::android::OpenApkAsset(kCfiFileName, &cfi_region);
-        DCHECK(fd >= 0);
-        bool mapped_file_ok =
-            chrome_cfi_file_.Initialize(base::File(fd), cfi_region);
-        DCHECK(mapped_file_ok);
-        chrome_cfi_table_ = base::ArmCFITable::Parse(
-            {chrome_cfi_file_.data(), chrome_cfi_file_.length()});
-        DCHECK(chrome_cfi_table_);
-      }
-      UnwindersFactory(const UnwindersFactory&) = delete;
-      UnwindersFactory& operator=(const UnwindersFactory&) = delete;
-
-      std::vector<std::unique_ptr<base::Unwinder>> Run() {
-        std::vector<std::unique_ptr<base::Unwinder>> unwinders;
-        unwinders.push_back(module_->CreateNativeUnwinder(
-            memory_regions_map_.get(),
-            reinterpret_cast<uintptr_t>(&__executable_start)));
-        unwinders.push_back(std::make_unique<base::ChromeUnwinderAndroid>(
-            chrome_cfi_table_.get(),
-            reinterpret_cast<uintptr_t>(&__executable_start)));
-        return unwinders;
-      }
-
-     private:
-      const std::unique_ptr<stack_unwinder::Module> module_;
-      const std::unique_ptr<stack_unwinder::MemoryRegionsMap>
-          memory_regions_map_;
-      base::MemoryMappedFile chrome_cfi_file_;
-      std::unique_ptr<base::ArmCFITable> chrome_cfi_table_;
-    };
-
-    return base::BindRepeating(&UnwindersFactory::Run,
-                               std::make_unique<UnwindersFactory>());
-#else
-    return base::BindRepeating(
-        []() -> std::vector<std::unique_ptr<base::Unwinder>> { return {}; });
-#endif
-  };
+// Encapsulates the setup required to create the Chrome unwinder on Android.
+class ChromeUnwinderCreator {
+ public:
+  ChromeUnwinderCreator() {
+    constexpr char kCfiFileName[] = "assets/unwind_cfi_32";
+
+    base::MemoryMappedFile::Region cfi_region;
+    int fd = base::android::OpenApkAsset(kCfiFileName, &cfi_region);
+    DCHECK(fd >= 0);
+    bool mapped_file_ok =
+        chrome_cfi_file_.Initialize(base::File(fd), cfi_region);
+    DCHECK(mapped_file_ok);
+    chrome_cfi_table_ = base::ArmCFITable::Parse(
+        {chrome_cfi_file_.data(), chrome_cfi_file_.length()});
+    DCHECK(chrome_cfi_table_);
+  }
 
-  static base::NoDestructor<
-      base::RepeatingCallback<std::vector<std::unique_ptr<base::Unwinder>>()>>
-      native_unwinder_factory(create_unwinders_factory());
+  ChromeUnwinderCreator(const ChromeUnwinderCreator&) = delete;
+  ChromeUnwinderCreator& operator=(const ChromeUnwinderCreator&) = delete;
+
+  std::unique_ptr<base::Unwinder> Create() {
+    return std::make_unique<base::ChromeUnwinderAndroid>(
+        chrome_cfi_table_.get(),
+        reinterpret_cast<uintptr_t>(&__executable_start));
+  }
+
+ private:
+  base::MemoryMappedFile chrome_cfi_file_;
+  std::unique_ptr<base::ArmCFITable> chrome_cfi_table_;
+};
 
-  return *native_unwinder_factory;
+// Encapsulates the setup required to create the Android native unwinder.
+class NativeUnwinderCreator {
+ public:
+  NativeUnwinderCreator()
+      : module_(stack_unwinder::Module::Load()),
+        memory_regions_map_(module_->CreateMemoryRegionsMap()) {}
+  NativeUnwinderCreator(const NativeUnwinderCreator&) = delete;
+  NativeUnwinderCreator& operator=(const NativeUnwinderCreator&) = delete;
+
+  std::unique_ptr<base::Unwinder> Create() {
+    return module_->CreateNativeUnwinder(
+        memory_regions_map_.get(),
+        reinterpret_cast<uintptr_t>(&__executable_start));
+  }
+
+ private:
+  const std::unique_ptr<stack_unwinder::Module> module_;
+  const std::unique_ptr<stack_unwinder::MemoryRegionsMap> memory_regions_map_;
+};
+
+// This function must only be called via the closure created in
+// CreateCoreUnwindersFactory(), which is passed into StackSamplingProfiler.
+// This ensures the expensive work involved in constructing the unwinder
+// creators is done on the profiler thread rather than the profiled thread.
+// These take 50+ ms to execute due to the creation of the memory regions map
+// and lookup of the modules in the process.
+std::vector<std::unique_ptr<base::Unwinder>> CreateCoreUnwinders() {
+  // This function is expected to be run on the profiler thread which is never
+  // the main thread in the process.
+  DCHECK_NE(getpid(), gettid());
+
+  static base::NoDestructor<NativeUnwinderCreator> native_unwinder_creator;
+  static base::NoDestructor<ChromeUnwinderCreator> chrome_unwinder_creator;
+
+  // Note order matters: the more general unwinder must appear first in the
+  // vector.
+  std::vector<std::unique_ptr<base::Unwinder>> unwinders;
+  unwinders.push_back(native_unwinder_creator->Create());
+  unwinders.push_back(chrome_unwinder_creator->Create());
+  return unwinders;
+}
+#endif  // defined(OS_ANDROID) && BUILDFLAG(ENABLE_ARM_CFI_TABLE)
+
+base::StackSamplingProfiler::UnwindersFactory CreateCoreUnwindersFactory() {
+#if defined(OS_ANDROID) && BUILDFLAG(ENABLE_ARM_CFI_TABLE)
+  // The module is loadable if the profiler is enabled for the current
+  // process.
+  CHECK(
+      ThreadProfilerConfiguration::Get()->IsProfilerEnabledForCurrentProcess());
+
+  return base::BindOnce(&CreateCoreUnwinders);
+#else
+  return base::StackSamplingProfiler::UnwindersFactory();
+#endif
 }
 
 const base::RepeatingClosure GetApplyPerSampleMetadataCallback(
@@ -354,7 +382,7 @@ ThreadProfiler::ThreadProfiler(
           CallStackProfileParams(process_, thread,
                                  CallStackProfileParams::PROCESS_STARTUP),
           work_id_recorder_.get()),
-      GetCoreUnwindersFactory().Run(),
+      CreateCoreUnwindersFactory(),
       GetApplyPerSampleMetadataCallback(process_));
 
   startup_profiler_->Start();
@@ -420,7 +448,7 @@ void ThreadProfiler::StartPeriodicSamplingCollection() {
           base::BindOnce(&ThreadProfiler::OnPeriodicCollectionCompleted,
                          owning_thread_task_runner_,
                          weak_factory_.GetWeakPtr())),
-      GetCoreUnwindersFactory().Run(),
+      CreateCoreUnwindersFactory(),
       GetApplyPerSampleMetadataCallback(process_));
   if (aux_unwinder_factory_)
     periodic_profiler_->AddAuxUnwinder(aux_unwinder_factory_.Run());

services/tracing/public/cpp/stack_sampling/tracing_sampler_profiler.cc

@@ -8,6 +8,7 @@
 #include <set>
 
 #include "base/android/library_loader/anchor_functions.h"
+#include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/debug/leak_annotations.h"
 #include "base/debug/stack_trace.h"
@@ -706,11 +707,14 @@ void TracingSamplerProfiler::StartTracing(
   // Create and start the stack sampling profiler.
 #if defined(OS_ANDROID)
 #if ANDROID_ARM64_UNWINDING_SUPPORTED
-  std::vector<std::unique_ptr<base::Unwinder>> unwinder;
-  unwinder.push_back(std::make_unique<UnwinderArm64>());
+  const auto create_unwinders = []() {
+    std::vector<std::unique_ptr<base::Unwinder>> unwinders;
+    unwinders.push_back(std::make_unique<UnwinderArm64>());
+    return unwinders;
+  };
   profiler_ = std::make_unique<base::StackSamplingProfiler>(
       sampled_thread_token_, params, std::move(profile_builder),
-      std::move(unwinder));
+      base::BindOnce(create_unwinders));
   profiler_->Start();
 
 #elif ANDROID_CFI_UNWINDING_SUPPORTED