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

base/profiler/native_stack_sampler.h

 // 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.
 
 #ifndef BASE_PROFILER_NATIVE_STACK_SAMPLER_H_
 #define BASE_PROFILER_NATIVE_STACK_SAMPLER_H_
 
 #include <memory>
 
 #include "base/base_export.h"
 #include "base/macros.h"
 #include "base/profiler/stack_sampling_profiler.h"
 #include "base/threading/platform_thread.h"
 
 namespace base {
 
 class NativeStackSamplerTestDelegate;
 
 // NativeStackSampler is an implementation detail of StackSamplingProfiler. It
 // abstracts the native implementation required to record a stack sample for a
 // given thread.
 class NativeStackSampler {
  public:
+  // This class contains data structures that can be shared across multiple
+  // instances of NativeStackSampler so long as they do not run concurrently.
+  // This supports having a single instance of large structures such as copy
+  // buffers.

[Mike Wittman, 2017/05/10 01:29:56]

For this comment I think it's sufficient to say just:

Holds a memory buffer for stack copies that may be shared across multiple
NativeStackSamplers.

[bcwhite, 2017/05/10 13:50:37]

The "no concurrent" clause is important.

[Mike Wittman, 2017/05/10 17:17:21]

"It's not safe to share a writable memory buffer across threads without
synchronization" is something we can expect a competent developer to know and
not worth restating in a comment. This is more likely to confuse than be helpful
because readers will either wonder why the comment is even there, or assume it's
giving something other than basic programming advice and seek to interpret
"concurrently" in some incorrect manner (for example: referring to the existing
interleaving of sample collection across multiple target threads).

It's also unlikely that concurrent profiling will ever be needed, so whether
it's safe to use the buffer concurrently is not something readers need to care
about.

(Applies to all comments warning about concurrency.)

[bcwhite, 2017/05/11 16:56:31]

Done.

+  class StackBuffer {
+   public:
+    StackBuffer(size_t buffer_size);
+    ~StackBuffer();
+
+    unsigned char* buffer() { return buffer_.get(); }

[Mike Wittman, 2017/05/10 01:29:57]

void* for return type. There's no natural size for the contents of this buffer,
and we shouldn't impose one arbitrarily.

[bcwhite, 2017/05/10 13:50:37]

Done.

+    size_t size() { return size_; }
+
+   private:
+    const std::unique_ptr<unsigned char[]> buffer_;
+    const size_t size_;
+
+    DISALLOW_COPY_AND_ASSIGN(StackBuffer);
+  };
+
   // The callback type used to add annotations to a sample during collection.
   // This is passed to the native sampler to be applied at the most appropriate
   // time. It is a simple function-pointer because the generated code must be
   // completely predictable and do nothing that could acquire a mutex; a
   // Callback object is code outside the control of this object and could,
   // for example, acquire a mutex as part of allocating memory for a LOG
   // message.
   using AnnotateCallback = void (*)(StackSamplingProfiler::Sample*);
 
   virtual ~NativeStackSampler();
 
   // Creates a stack sampler that records samples for |thread_handle|. Returns
   // null if this platform does not support stack sampling.
   static std::unique_ptr<NativeStackSampler> Create(
       PlatformThreadId thread_id,
       AnnotateCallback annotator,
       NativeStackSamplerTestDelegate* test_delegate);
 
+  // Creates an instance of the necessary stack buffer.
+  static std::unique_ptr<StackBuffer> CreateStackBuffer();
+
   // The following functions are all called on the SamplingThread (not the
   // thread being sampled).
 
   // Notifies the sampler that we're starting to record a new profile. Modules
   // shared across samples in the profile should be recorded in |modules|.
   virtual void ProfileRecordingStarting(
       std::vector<StackSamplingProfiler::Module>* modules) = 0;
 
   // Records a stack sample to |sample|.
-  virtual void RecordStackSample(StackSamplingProfiler::Sample* sample) = 0;
+  virtual void RecordStackSample(StackBuffer* stackbuffer,

[Mike Wittman, 2017/05/10 01:29:57]

nit: stack_buffer (applies throughout)

[bcwhite, 2017/05/10 13:50:37]

Done.

+                                 StackSamplingProfiler::Sample* sample) = 0;
 
   // Notifies the sampler that we've stopped recording the current
   // profile.
-  virtual void ProfileRecordingStopped() = 0;
+  virtual void ProfileRecordingStopped(StackBuffer* stackbuffer) = 0;
 
  protected:
   NativeStackSampler();
 
  private:
   DISALLOW_COPY_AND_ASSIGN(NativeStackSampler);
 };
 
 // NativeStackSamplerTestDelegate provides seams for test code to execute during
 // stack collection.
 class BASE_EXPORT NativeStackSamplerTestDelegate {
  public:
   virtual ~NativeStackSamplerTestDelegate();
 
   // Called after copying the stack and resuming the target thread, but prior to
   // walking the stack. Invoked on the SamplingThread.
   virtual void OnPreStackWalk() = 0;
 
  protected:
   NativeStackSamplerTestDelegate();
 
  private:
   DISALLOW_COPY_AND_ASSIGN(NativeStackSamplerTestDelegate);
 };
 
 }  // namespace base
 
 #endif  // BASE_PROFILER_NATIVE_STACK_SAMPLER_H_