Use a common buffer across all instances for stack-copy.

base/profiler/native_stack_sampler_win.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/profiler/native_stack_sampler.h"
 
 #include <objbase.h>
 #include <windows.h>
 #include <stddef.h>
 #include <winternl.h>
 
 #include <cstdlib>
 #include <map>
 #include <memory>
 #include <utility>
 #include <vector>
 
+#include "base/atomicops.h"

[Mike Wittman, 2017/01/24 17:20:57]

No longer needed.

[bcwhite, 2017/02/07 15:16:36]

Done.

 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/profiler/win32_stack_frame_unwinder.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/synchronization/lock.h"

[Mike Wittman, 2017/01/24 17:20:57]

No longer needed.

[bcwhite, 2017/02/07 15:16:36]

Done.

 #include "base/time/time.h"
 #include "base/win/pe_image.h"
 #include "base/win/scoped_handle.h"
 
 namespace base {
 
 // Stack recording functions --------------------------------------------------
 
 namespace {
 
@@ skipping 350 matching lines @@
 // NativeStackSamplerWin ------------------------------------------------------
 
 class NativeStackSamplerWin : public NativeStackSampler {
  public:
   NativeStackSamplerWin(win::ScopedHandle thread_handle,
                         AnnotateCallback annotator,
                         NativeStackSamplerTestDelegate* test_delegate);
   ~NativeStackSamplerWin() override;
 
   // StackSamplingProfiler::NativeStackSampler:
+  std::unique_ptr<Common> CreateCommon() override;
   void ProfileRecordingStarting(
+      Common* common,
       std::vector<StackSamplingProfiler::Module>* modules) override;
-  void RecordStackSample(StackSamplingProfiler::Sample* sample) override;
-  void ProfileRecordingStopped() override;
+  void RecordStackSample(Common* common,
+                         StackSamplingProfiler::Sample* sample) override;
+  void ProfileRecordingStopped(Common* common) override;
 
  private:
   enum {
     // Intended to hold the largest stack used by Chrome. The default Win32
     // reserved stack size is 1 MB and Chrome Windows threads currently always
     // use the default, but this allows for expansion if it occurs. The size
     // beyond the actual stack size consists of unallocated virtual memory pages
     // so carries little cost (just a bit of wasted address space).
     kStackCopyBufferSize = 2 * 1024 * 1024
   };
 
+  class WinCommon : public Common {

[Mike Wittman, 2017/01/24 17:20:57]

WinCommon => CommonWin (or actually StackBufferWin per previous comment)

Convention in Chrome is to use the platform specific name as suffix.

[bcwhite, 2017/02/07 15:16:35]

Done.

+   public:
+    WinCommon() {}
+    ~WinCommon() override {}
+
+    // Buffer to use for copies of the stack.
+    unsigned char stack_copy_buffer_[kStackCopyBufferSize];
+  };
+
   // Attempts to query the module filename, base address, and id for
   // |module_handle|, and store them in |module|. Returns true if it succeeded.
   static bool GetModuleForHandle(HMODULE module_handle,
                                  StackSamplingProfiler::Module* module);
 
   // Gets the index for the Module corresponding to |module_handle| in
   // |modules|, adding it if it's not already present. Returns
   // StackSamplingProfiler::Frame::kUnknownModuleIndex if no Module can be
   // determined for |module|.
   size_t GetModuleIndex(HMODULE module_handle,
                         std::vector<StackSamplingProfiler::Module>* modules);
 
   // Copies the information represented by |stack| into |sample| and |modules|.
   void CopyToSample(const std::vector<RecordedFrame>& stack,
                     StackSamplingProfiler::Sample* sample,
                     std::vector<StackSamplingProfiler::Module>* modules);
 
   win::ScopedHandle thread_handle_;
 
   const AnnotateCallback annotator_;
 
   NativeStackSamplerTestDelegate* const test_delegate_;
 
   // The stack base address corresponding to |thread_handle_|.
   const void* const thread_stack_base_address_;
 
-  // Buffer to use for copies of the stack. We use the same buffer for all the
-  // samples to avoid the overhead of multiple allocations and frees.
-  const std::unique_ptr<unsigned char[]> stack_copy_buffer_;
-
   // Weak. Points to the modules associated with the profile being recorded
   // between ProfileRecordingStarting() and ProfileRecordingStopped().
   std::vector<StackSamplingProfiler::Module>* current_modules_;
 
   // Maps a module handle to the corresponding Module's index within
   // current_modules_.
   std::map<HMODULE, size_t> profile_module_index_;
 
+  // This "in use" flag is used to validate the assumption that there are no
+  // concurrent stack sampling requests regardless of how many instances of
+  // this class are currently active.
+  static subtle::AtomicWord stack_copy_buffer_in_use_;

[Mike Wittman, 2017/01/24 17:20:57]

No longer needed.

[bcwhite, 2017/02/07 15:16:35]

Done.

+
   DISALLOW_COPY_AND_ASSIGN(NativeStackSamplerWin);
 };
 
 NativeStackSamplerWin::NativeStackSamplerWin(
     win::ScopedHandle thread_handle,
     AnnotateCallback annotator,
     NativeStackSamplerTestDelegate* test_delegate)
     : thread_handle_(thread_handle.Take()),
       annotator_(annotator),
       test_delegate_(test_delegate),
       thread_stack_base_address_(
-          GetThreadEnvironmentBlock(thread_handle_.Get())->Tib.StackBase),
-      stack_copy_buffer_(new unsigned char[kStackCopyBufferSize]) {
+          GetThreadEnvironmentBlock(thread_handle_.Get())->Tib.StackBase) {
   DCHECK(annotator_);
 }
 
 NativeStackSamplerWin::~NativeStackSamplerWin() {
 }
 
+std::unique_ptr<NativeStackSampler::Common>
+NativeStackSamplerWin::CreateCommon() {
+  return MakeUnique<WinCommon>();
+}
+
 void NativeStackSamplerWin::ProfileRecordingStarting(
+    Common* common,
     std::vector<StackSamplingProfiler::Module>* modules) {
   current_modules_ = modules;
   profile_module_index_.clear();
 }
 
 void NativeStackSamplerWin::RecordStackSample(
+    Common* common,
     StackSamplingProfiler::Sample* sample) {
+  DCHECK(common);
   DCHECK(current_modules_);
 
-  if (!stack_copy_buffer_)
-    return;
-
   std::vector<RecordedFrame> stack;
-  SuspendThreadAndRecordStack(thread_handle_.Get(), thread_stack_base_address_,
-                              stack_copy_buffer_.get(), kStackCopyBufferSize,
-                              &stack, annotator_, sample, test_delegate_);
+  SuspendThreadAndRecordStack(
+      thread_handle_.Get(), thread_stack_base_address_,
+      reinterpret_cast<WinCommon*>(common)->stack_copy_buffer_,

[Mike Wittman, 2017/01/24 17:20:57]

static_cast for downcast

[bcwhite, 2017/02/07 15:16:36]

Done.

+      kStackCopyBufferSize, &stack, annotator_, sample, test_delegate_);
   CopyToSample(stack, sample, current_modules_);
 }
 
-void NativeStackSamplerWin::ProfileRecordingStopped() {
+void NativeStackSamplerWin::ProfileRecordingStopped(Common* common) {
   current_modules_ = nullptr;
 }
 
 // static
 bool NativeStackSamplerWin::GetModuleForHandle(
     HMODULE module_handle,
     StackSamplingProfiler::Module* module) {
   wchar_t module_name[MAX_PATH];
   DWORD result_length =
       GetModuleFileName(module_handle, module_name, arraysize(module_name));
@@ skipping 59 matching lines @@
 
   if (thread_handle) {
     return std::unique_ptr<NativeStackSampler>(new NativeStackSamplerWin(
         win::ScopedHandle(thread_handle), annotator, test_delegate));
   }
 #endif
   return std::unique_ptr<NativeStackSampler>();
 }
 
 }  // namespace base