Add process lifetime annotations to stack samples.

base/profiler/stack_sampling_profiler.h

Index: base/profiler/stack_sampling_profiler.h
diff --git a/base/profiler/stack_sampling_profiler.h b/base/profiler/stack_sampling_profiler.h
index bf5a4f337219f76b782189fde2c5aa2f9646c640..11592288e0b78d538d642e0c90e4ef42c679d02c 100644
--- a/base/profiler/stack_sampling_profiler.h
+++ b/base/profiler/stack_sampling_profiler.h
@@ -11,6 +11,7 @@
 #include <string>
 #include <vector>
 
+#include "base/atomicops.h"
 #include "base/base_export.h"
 #include "base/callback.h"
 #include "base/files/file_path.h"
@@ -64,6 +65,15 @@ class NativeStackSamplerTestDelegate;
 // record those modules associated with the recorded stack frames.
 class BASE_EXPORT StackSamplingProfiler {
  public:
+  enum ProcessPhase : int {

[Mike Wittman, 2016/10/24 22:28:14]

uint32_t (and below)

[bcwhite, 2016/10/25 14:45:23]

I'd rather not since I'm using it as a "flag number" that's only coded as "1 <<
flag".  "int" is the natural word type on the machine.

[Mike Wittman, 2016/10/25 17:24:22]

Ah, right. Forgot that this was an index and not a mask.

Do we even need a type specifier here?

[bcwhite, 2016/10/25 21:10:42]

I assumed it was necessary in order to use it as an integer with the "<<"
operator...  but it turns out not.  Removed.

+    // TODO: Expand this.
+    FirstNonEmptyPaint,
+  };
+
+  enum ProcessActivity : int {
+    // TODO: Expand this, too.
+  };
+
   // Module represents the module (DLL or exe) corresponding to a stack frame.
   struct BASE_EXPORT Module {
     Module();
@@ -107,8 +117,20 @@ class BASE_EXPORT StackSamplingProfiler {
     size_t module_index;
   };
 
-  // Sample represents a set of stack frames.
-  using Sample = std::vector<Frame>;
+  // Sample represents a set of stack frames with some extra information.
+  struct BASE_EXPORT Sample {

[Mike Wittman, 2016/10/24 22:28:14]

We probably should a add constructor at this point to ensure the bitfields are
initialized. That should reduce the verbosity of the test code as well.

[bcwhite, 2016/10/25 14:45:23]

Done.

+    // The entire stack frame when the sample is taken.
+    std::vector<Frame> frames;
+
+    // A bit-field indicating which process phases have passed. This can be
+    // used to tell where in the process lifetime the samples are taken. See
+    // ProcessPhase, above.
+    int32_t process_phases;

[Mike Wittman, 2016/10/24 22:28:14]

uint32_t (and below)

[bcwhite, 2016/10/25 14:45:23]

Atomic32 is always a signed (unfortunately) so making this signed allows
conversion without risking a bunch of signed/unsigned compiler gymnastics.

[Mike Wittman, 2016/10/25 17:24:22]

The use of Atomic32 is an implementation detail of the profiler; I don't think
it should influence the interfaces.

The unsigned cast is guaranteed to preserve the bit pattern, and it looks like
there's only two places where it would be required, so this doesn't seem like a
significant burden.

[bcwhite, 2016/10/25 21:10:42]

Good enough for me.  Perhaps the compilers will be generous given that it's only
assignments.

+
+    // A bit-field indicating activities which were active when the frame was
+    // captured. See ProcessActivity, above.
+    int32_t current_activities;
+  };
 
   // CallStackProfile represents a set of samples.
   struct BASE_EXPORT CallStackProfile {
@@ -189,6 +211,12 @@ class BASE_EXPORT StackSamplingProfiler {
   // whichever occurs first.
   void Stop();
 
+  // Set the current system state that is recorded with each captured stack
+  // frame.
+  static void SetProcessPhase(ProcessPhase phase);
+  static void RecordActivityBegin(ProcessActivity activity);
+  static void RecordActivityEnd(ProcessActivity activity);
+
  private:
   // SamplingThread is a separate thread used to suspend and sample stacks from
   // the target thread.
@@ -231,6 +259,11 @@ class BASE_EXPORT StackSamplingProfiler {
     DISALLOW_COPY_AND_ASSIGN(SamplingThread);
   };
 
+  // These global variables hold current system state. These values are
+  // recorded with every captured frame.
+  static volatile subtle::Atomic32 process_phases_;

[Mike Wittman, 2016/10/24 22:28:14]

I believe declaring subtle::Atomic32 volatile is unnecessary. It's not done
elsewhere in Chrome.

[bcwhite, 2016/10/25 14:45:23]

True.  It is done in at least one place (because I did it) but its only a
requirement if its an atomic shared across different processes.

+  static volatile subtle::Atomic32 current_activities_;
+
   // The thread whose stack will be sampled.
   PlatformThreadId thread_id_;
 
@@ -251,6 +284,12 @@ class BASE_EXPORT StackSamplingProfiler {
 // done in tests and by the metrics provider code.
 BASE_EXPORT bool operator==(const StackSamplingProfiler::Module& a,
                             const StackSamplingProfiler::Module& b);
+BASE_EXPORT bool operator==(const StackSamplingProfiler::Sample& a,
+                            const StackSamplingProfiler::Sample& b);
+BASE_EXPORT bool operator!=(const StackSamplingProfiler::Sample& a,
+                            const StackSamplingProfiler::Sample& b);
+BASE_EXPORT bool operator<(const StackSamplingProfiler::Sample& a,
+                           const StackSamplingProfiler::Sample& b);
 BASE_EXPORT bool operator==(const StackSamplingProfiler::Frame& a,
                             const StackSamplingProfiler::Frame& b);
 BASE_EXPORT bool operator<(const StackSamplingProfiler::Frame& a,