Support for extracting user-data from activity tracking.

base/debug/activity_tracker.h

Index: base/debug/activity_tracker.h
diff --git a/base/debug/activity_tracker.h b/base/debug/activity_tracker.h
index a3a9419c7ab1eb01a6edc0f9f4bab0bb72db5b7c..e16e1887f504538ad747b06d77b7a811937be3a5 100644
--- a/base/debug/activity_tracker.h
+++ b/base/debug/activity_tracker.h
@@ -155,6 +155,18 @@ class ActivityTrackerMemoryAllocator {
   // Returns an object to the "free" pool.
   void ReleaseObjectReference(Reference ref);
 
+  // Helper function to access an object allocated using this instance.
+  template <typename T>
+  T* GetAsObject(Reference ref) {
+    return allocator_->GetAsObject<T>(ref, object_type_);
+  }
+
+  // Similar to GetAsObject() but converts references to arrays of objects.
+  template <typename T>
+  T* GetAsArray(Reference ref, size_t count) {
+    return allocator_->GetAsArray<T>(ref, object_type_, count);
+  }
+
   // The current "used size" of the internal cache, visible for testing.
   size_t cache_used() const { return cache_used_; }
 
@@ -244,8 +256,10 @@ struct Activity {
   // enabled.
   uint64_t call_stack[kActivityCallStackSize];
 
-  // Reference to arbitrary user data within the persistent memory segment.
-  uint32_t user_data;
+  // Reference to arbitrary user data within the persistent memory segment
+  // and a unique identifier for it.
+  uint32_t user_data_ref;
+  uint32_t user_data_id;
 
   // The (enumerated) type of the activity. This defines what fields of the
   // |data| record are valid.
@@ -254,7 +268,7 @@ struct Activity {
   // Padding to ensure that the next member begins on a 64-bit boundary
   // even on 32-bit builds which ensures inter-operability between CPU
   // architectures. New fields can be taken from this space.
-  uint8_t padding[3];
+  uint8_t padding[7];
 
   // Information specific to the |activity_type|.
   ActivityData data;
@@ -266,40 +280,12 @@ struct Activity {
                        const ActivityData& data);
 };
 
-// This structure holds a copy of all the internal data at the moment the
-// "snapshot" operation is done. It is disconnected from the live tracker
-// so that continued operation of the thread will not cause changes here.
-struct BASE_EXPORT ActivitySnapshot {
-  // Explicit constructor/destructor are needed because of complex types
-  // with non-trivial default constructors and destructors.
-  ActivitySnapshot();
-  ~ActivitySnapshot();
-
-  // The name of the thread as set when it was created. The name may be
-  // truncated due to internal length limitations.
-  std::string thread_name;
-
-  // The process and thread IDs. These values have no meaning other than
-  // they uniquely identify a running process and a running thread within
-  // that process.  Thread-IDs can be re-used across different processes
-  // and both can be re-used after the process/thread exits.
-  int64_t process_id = 0;
-  int64_t thread_id = 0;
-
-  // The current stack of activities that are underway for this thread. It
-  // is limited in its maximum size with later entries being left off.
-  std::vector<Activity> activity_stack;
-
-  // The current total depth of the activity stack, including those later
-  // entries not recorded in the |activity_stack| vector.
-  uint32_t activity_stack_depth = 0;
-};
-
 // This class manages arbitrary user data that can be associated with activities
 // done by a thread by supporting key/value pairs of any type. This can provide
 // additional information during debugging. It is also used to store arbitrary
 // global data. All updates must be done from the same thread.
 class BASE_EXPORT ActivityUserData {
+ public:
   // List of known value type. REFERENCE types must immediately follow the non-
   // external types.
   enum ValueType : uint8_t {
@@ -309,14 +295,47 @@ class BASE_EXPORT ActivityUserData {
     STRING_VALUE,
     STRING_VALUE_REFERENCE,
     CHAR_VALUE,
+    BOOL_VALUE,
     SIGNED_VALUE,
     UNSIGNED_VALUE,
   };
 
- public:
+  class BASE_EXPORT TypedValue {
+   public:
+    TypedValue();
+    TypedValue(const TypedValue& other);
+    ~TypedValue();
+
+    StringPiece Get() const;
+    StringPiece GetReference() const;
+    StringPiece GetString() const;
+    StringPiece GetStringReference() const;
+    bool GetBool() const;
+    char GetChar() const;
+    int64_t GetInt() const;
+    uint64_t GetUint() const;
+
+   private:
+    friend class ActivityUserData;
+
+    ValueType type;
+    uint64_t short_value;    // Used to hold copy of numbers, etc.
+    std::string long_value;  // Used to hold copy of raw/string data.
+    StringPiece ref_value;   // Used to hold reference to external data.

[manzagop (departed), 2016/11/29 22:42:58]

I wonder if using StringPiece is risky/error prone wrt thinking it actually
refers to memory in the current process. Wdyt?

[bcwhite, 2016/12/01 16:29:39]

It would certainly be a problem if any string operations were done on it and the
reference was not valid.  But I think that's the same no matter what
representation is used.  StringPiece is basically a pair<void*, size_t> with
helpers.

I'll add a comment.

+  };
+
+  using Snapshot = std::map<std::string, TypedValue>;
+
   ActivityUserData(void* memory, size_t size);
   ~ActivityUserData();
 
+  // Gets the unique ID number for this user data. If this changes then the
+  // contents have been overwritten by another thread. The return value is
+  // always non-zero unless it's actually just a data "sink".
+  uint32_t id() const {
+    return memory_ ? id_->load(std::memory_order_relaxed) : 0;
+  }
+
   // Writes a |value| (as part of a key/value pair) that will be included with
   // the activity in any reports. The same |name| can be written multiple times
   // with each successive call overwriting the previously stored |value|. For
@@ -332,6 +351,10 @@ class BASE_EXPORT ActivityUserData {
   void SetString(StringPiece name, StringPiece value) {
     Set(name, STRING_VALUE, value.data(), value.length());
   }
+  void SetBool(StringPiece name, bool value) {
+    char cvalue = value ? 1 : 0;
+    Set(name, BOOL_VALUE, &cvalue, sizeof(cvalue));
+  }
   void SetChar(StringPiece name, char value) {
     Set(name, CHAR_VALUE, &value, sizeof(value));
   }
@@ -353,6 +376,13 @@ class BASE_EXPORT ActivityUserData {
     SetReference(name, STRING_VALUE_REFERENCE, value.data(), value.length());
   }
 
+  // Create a snapshot of the key/value pairs contained within. The returned
+  // data will be fixed, independent of whatever changes afterward. There is
+  // protection against concurrent modification of the values but no protection
+  // against a complete overwrite of the contents; the caller must ensure that
+  // the memory segment is not going to be re-initialized while this runs.
+  bool CreateSnapshot(Snapshot* output_snapshot) const;
+
  private:
   FRIEND_TEST_ALL_PREFIXES(ActivityTrackerTest, UserDataTest);
 
@@ -392,18 +422,62 @@ class BASE_EXPORT ActivityUserData {
                     const void* memory,
                     size_t size);
 
-  // TODO(bcwhite): Add Get() methods for Analyzer to use.
+  // Loads any data already in the memory segment. This allows for accessing
+  // records created previously.
+  void ImportExistingData() const;
+
+  // A map of all the values within the memory block, keyed by name for quick
+  // updates of the values. This is "mutable" because it changes on "const"
+  // objects even when the actual data values can't change.
+  mutable std::map<StringPiece, ValueInfo> values_;
 
-  std::map<StringPiece, ValueInfo> values_;
+  // Information about the memory block in which new data can be stored. These
+  // are "mutable" because they change even on "const" objects that are just
+  // skipping already set values.
+  mutable char* memory_;
+  mutable size_t available_;
 
-  char* memory_;
-  size_t available_;
+  // A pointer to the unique ID for this instance.
+  std::atomic<uint32_t>* const id_;
 
   base::ThreadChecker thread_checker_;
 
+  // This ID is used to create unique indentifiers for user data so that it's
+  // possible to tell if the information has been overwritten.
+  static std::atomic<uint32_t> next_id_;
+
   DISALLOW_COPY_AND_ASSIGN(ActivityUserData);
 };
 
+// This structure holds a copy of all the internal data at the moment the
+// "snapshot" operation is done. It is disconnected from the live tracker
+// so that continued operation of the thread will not cause changes here.
+struct BASE_EXPORT ActivitySnapshot {
+  // Explicit constructor/destructor are needed because of complex types
+  // with non-trivial default constructors and destructors.
+  ActivitySnapshot();
+  ~ActivitySnapshot();
+
+  // The name of the thread as set when it was created. The name may be
+  // truncated due to internal length limitations.
+  std::string thread_name;
+
+  // The process and thread IDs. These values have no meaning other than
+  // they uniquely identify a running process and a running thread within
+  // that process.  Thread-IDs can be re-used across different processes
+  // and both can be re-used after the process/thread exits.
+  int64_t process_id = 0;
+  int64_t thread_id = 0;
+
+  // The current stack of activities that are underway for this thread. It
+  // is limited in its maximum size with later entries being left off.
+  std::vector<Activity> activity_stack;
+
+  // The current total depth of the activity stack, including those later
+  // entries not recorded in the |activity_stack| vector.
+  uint32_t activity_stack_depth = 0;
+};
+
 // This class manages tracking a stack of activities for a single thread in
 // a persistent manner, implementing a bounded-size stack in a fixed-size
 // memory allocation. In order to support an operational mode where another
@@ -438,10 +512,7 @@ class BASE_EXPORT ThreadActivityTracker {
     // Changes some basic metadata about the activity.
     void ChangeTypeAndData(Activity::Type type, const ActivityData& data);
 
-    // Returns an object for manipulating user data.
-    ActivityUserData& user_data();
-
-   private:
+   protected:
     // The thread tracker to which this object reports. It can be null if
     // activity tracking is not (yet) enabled.
     ThreadActivityTracker* const tracker_;
@@ -449,9 +520,6 @@ class BASE_EXPORT ThreadActivityTracker {
     // An identifier that indicates a specific activity on the stack.
     ActivityId activity_id_;
 
-    // An object that manages additional user data, created only upon request.
-    std::unique_ptr<ActivityUserData> user_data_;
-
     DISALLOW_COPY_AND_ASSIGN(ScopedActivity);
   };
 
@@ -495,8 +563,18 @@ class BASE_EXPORT ThreadActivityTracker {
   // Indicates that an activity has completed.
   void PopActivity(ActivityId id);
 
-  // Returns an object capable of storing arbitrary user data.
-  std::unique_ptr<ActivityUserData> GetUserData(ActivityId id);
+  // Sets the user-data information for an activity.
+  std::unique_ptr<ActivityUserData> GetUserData(
+      ActivityId id,
+      ActivityTrackerMemoryAllocator* allocator);
+
+  // Returns if there is true use-data associated with a given ActivityId since
+  // it's possible than any returned object is just a sink.
+  bool HasUserData(ActivityId id);
+
+  // Release the user-data information for an activity.
+  void ReleaseUserData(ActivityId id,
+                       ActivityTrackerMemoryAllocator* allocator);
 
   // Returns whether the current data is valid or not. It is not valid if
   // corruption has been detected in the header or other data structures.
@@ -506,7 +584,7 @@ class BASE_EXPORT ThreadActivityTracker {
   // snapshot was not possible, perhaps because the data is not valid; the
   // contents of |output_snapshot| are undefined in that case. The current
   // implementation does not support concurrent snapshot operations.
-  bool Snapshot(ActivitySnapshot* output_snapshot) const;
+  bool CreateSnapshot(ActivitySnapshot* output_snapshot) const;
 
   // Calculates the memory size required for a given stack depth, including
   // the internal header structure for the stack.
@@ -539,9 +617,9 @@ class BASE_EXPORT GlobalActivityTracker {
   // will be safely ignored. These are public so that an external process
   // can recognize records of this type within an allocator.
   enum : uint32_t {
-    kTypeIdActivityTracker = 0x5D7381AF + 3,   // SHA1(ActivityTracker) v3
-    kTypeIdUserDataRecord = 0x615EDDD7 + 1,    // SHA1(UserDataRecord) v1
-    kTypeIdGlobalDataRecord = 0xAFE61ABE + 1,  // SHA1(GlobalDataRecord) v1
+    kTypeIdActivityTracker = 0x5D7381AF + 3,  // SHA1(ActivityTracker) v3
+    kTypeIdUserDataRecord = 0x615EDDD7 + 2,   // SHA1(UserDataRecord) v2
+    kTypeIdGlobalDataRecord = kTypeIdUserDataRecord + 0x5F1184F7,  // Global

[manzagop (departed), 2016/11/29 22:42:58]

nit: "SHA1(Global)" for consistency

[bcwhite, 2016/12/01 16:29:39]

Makes the line 82 characters.  :-(

 
     kTypeIdActivityTrackerFree = ~kTypeIdActivityTracker,
     kTypeIdUserDataRecordFree = ~kTypeIdUserDataRecord,
@@ -558,13 +636,11 @@ class BASE_EXPORT GlobalActivityTracker {
                          const void* origin,
                          Activity::Type type,
                          const ActivityData& data,
-                         bool lock_allowed)
-        : ThreadActivityTracker::ScopedActivity(
-              GetOrCreateTracker(lock_allowed),
-              program_counter,
-              origin,
-              type,
-              data) {}
+                         bool lock_allowed);
+    ~ScopedThreadActivity();
+
+    // Returns an object for manipulating user data.
+    ActivityUserData& user_data();
 
    private:
     // Gets (or creates) a tracker for the current thread. If locking is not
@@ -582,6 +658,9 @@ class BASE_EXPORT GlobalActivityTracker {
         return global_tracker->GetTrackerForCurrentThread();
     }
 
+    // An object that manages additional user data, created only upon request.
+    std::unique_ptr<ActivityUserData> user_data_;
+
     DISALLOW_COPY_AND_ASSIGN(ScopedThreadActivity);
   };
 
@@ -645,18 +724,11 @@ class BASE_EXPORT GlobalActivityTracker {
   // Releases the activity-tracker for the current thread (for testing only).
   void ReleaseTrackerForCurrentThreadForTesting();
 
-  // Gets a reference to memory for holding user-defined activity data. If
-  // the reference is valid, it's memory will be returned. If not, then a
-  // new reference will be created (and stored) and that memory returned.
-  void* GetUserDataMemory(PersistentMemoryAllocator::Reference* reference);
-
-  // Releases memory for user-defined activity data.
-  void ReleaseUserDataMemory(PersistentMemoryAllocator::Reference* reference);
-
   // Accesses the global data record for storing arbitrary key/value pairs.
   ActivityUserData& user_data() { return user_data_; }
 
  private:
+  friend class ScopedThreadActivity;
   friend class ActivityTrackerTest;
 
   enum : int {