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

base/profiler/native_stack_sampler.h

Index: base/profiler/native_stack_sampler.h
diff --git a/base/profiler/native_stack_sampler.h b/base/profiler/native_stack_sampler.h
index 8d7e441b69306b072ed67321a531ac76aee4e28c..7776e3d77a937cd8d387acb21d2cd93e8700be13 100644
--- a/base/profiler/native_stack_sampler.h
+++ b/base/profiler/native_stack_sampler.h
@@ -21,6 +21,25 @@ class NativeStackSamplerTestDelegate;
 // 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
@@ -39,6 +58,9 @@ class NativeStackSampler {
       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).
 
@@ -48,11 +70,12 @@ class NativeStackSampler {
       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();