[Sampling profiler] Upload metadata keys

Propagates the optional metadata key values to the metrics upload proto.

Bug: 976864
Change-Id: I1d3f4323dfa2a6643b599a0afeb25162b58b83c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1895969
Commit-Queue: Mike Wittman <wittman@chromium.org>
Reviewed-by: Charlie Andrews <charliea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712343}

components/metrics/call_stack_profile_builder.cc

@@ -47,56 +47,6 @@ uint64_t HashModuleFilename(const base::FilePath& filename) {
   return base::HashMetricName(name_bytes);
 }
 
-std::map<uint64_t, int64_t> CreateMetadataMap(
-    base::ProfileBuilder::MetadataItemArray items,
-    size_t item_count) {
-  std::map<uint64_t, int64_t> item_map;
-  for (size_t i = 0; i < item_count; ++i) {
-    item_map[items[i].name_hash] = items[i].value;
-  }
-  return item_map;
-}
-
-// Returns all metadata items with new values in the current sample.
-std::map<uint64_t, int64_t> GetNewOrModifiedMetadataItems(
-    const std::map<uint64_t, int64_t>& current_items,
-    const std::map<uint64_t, int64_t>& previous_items) {
-  std::map<uint64_t, int64_t> new_or_modified_items;
-  // By default, std::pairs are sorted by the first then second pair elements
-  // and therefore pairs with either element differing are treated as different.
-  std::set_difference(
-      current_items.begin(), current_items.end(), previous_items.begin(),
-      previous_items.end(),
-      std::inserter(new_or_modified_items, new_or_modified_items.begin()));
-  return new_or_modified_items;
-}
-
-// Returns all metadata items deleted since the previous sample.
-std::map<uint64_t, int64_t> GetDeletedMetadataItems(
-    const std::map<uint64_t, int64_t>& current_items,
-    const std::map<uint64_t, int64_t>& previous_items) {
-  std::map<uint64_t, int64_t> deleted_items;
-  // By default, std::pairs are sorted by the first then second pair elements
-  // and therefore pairs with either element differing are treated as different.
-  //
-  // To find removed items, we need to override this comparator to do a set
-  // subtraction based only on the item name hashes, ignoring the item values.
-  //
-  // The set_difference algorithm requires that the items in the set already be
-  // sorted according to whatever comparator is passed to set_difference.
-  // Because our new sort order is just a looser version of the existing set
-  // sort order, we can find the set_difference here without creating a new set.
-  auto name_hash_comparator = [](const std::pair<uint64_t, int64_t>& lhs,
-                                 const std::pair<uint64_t, int64_t>& rhs) {
-    return lhs.first < rhs.first;
-  };
-  std::set_difference(previous_items.begin(), previous_items.end(),
-                      current_items.begin(), current_items.end(),
-                      std::inserter(deleted_items, deleted_items.begin()),
-                      name_hash_comparator);
-  return deleted_items;
-}
-
 }  // namespace
 
 CallStackProfileBuilder::CallStackProfileBuilder(
@@ -273,26 +223,99 @@ bool CallStackProfileBuilder::StackComparer::operator()(
       });
 }
 
+bool CallStackProfileBuilder::MetadataKeyCompare::operator()(
+    const MetadataKey& a,
+    const MetadataKey& b) const {
+  return std::tie(a.name_hash, a.key) < std::tie(b.name_hash, b.key);
+}
+
+CallStackProfileBuilder::MetadataKey::MetadataKey(uint64_t name_hash,
+                                                  base::Optional<int64_t> key)
+    : name_hash(name_hash), key(key) {}
+
+CallStackProfileBuilder::MetadataKey::MetadataKey(const MetadataKey& other) =
+    default;
+CallStackProfileBuilder::MetadataKey& CallStackProfileBuilder::MetadataKey::
+operator=(const MetadataKey& other) = default;
+
+CallStackProfileBuilder::MetadataMap CallStackProfileBuilder::CreateMetadataMap(
+    base::ProfileBuilder::MetadataItemArray items,
+    size_t item_count) {
+  MetadataMap item_map;
+  for (size_t i = 0; i < item_count; ++i)
+    item_map[MetadataKey{items[i].name_hash, items[i].key}] = items[i].value;
+  return item_map;
+}
+
+CallStackProfileBuilder::MetadataMap
+CallStackProfileBuilder::GetNewOrModifiedMetadataItems(
+    const MetadataMap& current_items,
+    const MetadataMap& previous_items) {
+  MetadataMap new_or_modified_items;
+  // Find the new or modified items by subtracting any previous items that are
+  // exactly the same as the current items (i.e. equal in key *and* value).
+  auto key_and_value_comparator = [](const std::pair<MetadataKey, int64_t>& a,
+                                     const std::pair<MetadataKey, int64_t>& b) {
+    return std::tie(a.first.name_hash, a.first.key, a.second) <
+           std::tie(b.first.name_hash, b.first.key, b.second);
+  };
+  std::set_difference(
+      current_items.begin(), current_items.end(), previous_items.begin(),
+      previous_items.end(),
+      std::inserter(new_or_modified_items, new_or_modified_items.begin()),
+      key_and_value_comparator);
+  return new_or_modified_items;
+}
+
+CallStackProfileBuilder::MetadataMap
+CallStackProfileBuilder::GetDeletedMetadataItems(
+    const MetadataMap& current_items,
+    const MetadataMap& previous_items) {
+  MetadataMap deleted_items;
+  // Find the deleted metadata items by subtracting the current items from the
+  // previous items, comparing items solely map key (as opposed to map key and
+  // value as in GetNewOrModifiedMetadataItems()). Comparing by key is necessary
+  // to distinguish modified items from deleted items: subtraction of modified
+  // items, which have the same key but different values, should produce the
+  // empty set. Deleted items have a key only in |previous_items| so should be
+  // retained in the result.
+  auto key_comparator = [](const std::pair<MetadataKey, int64_t>& lhs,
+                           const std::pair<MetadataKey, int64_t>& rhs) {
+    return MetadataKeyCompare()(lhs.first, rhs.first);
+  };
+  std::set_difference(previous_items.begin(), previous_items.end(),
+                      current_items.begin(), current_items.end(),
+                      std::inserter(deleted_items, deleted_items.begin()),
+                      key_comparator);
+  return deleted_items;
+}
+
 void CallStackProfileBuilder::AddSampleMetadata(
     CallStackProfile* profile,
     CallStackProfile::StackSample* sample) {
-  std::map<uint64_t, int64_t> current_items =
+  MetadataMap current_items =
       CreateMetadataMap(metadata_items_, metadata_item_count_);
 
   for (auto item :
        GetNewOrModifiedMetadataItems(current_items, previous_items_)) {
-    size_t name_hash_index = MaybeAddNameHashToProfile(profile, item.first);
+    size_t name_hash_index =
+        MaybeAddNameHashToProfile(profile, item.first.name_hash);
 
     CallStackProfile::MetadataItem* profile_item = sample->add_metadata();
     profile_item->set_name_hash_index(name_hash_index);
+    if (item.first.key.has_value())
+      profile_item->set_key(*item.first.key);
     profile_item->set_value(item.second);
   }
 
   for (auto item : GetDeletedMetadataItems(current_items, previous_items_)) {
-    size_t name_hash_index = MaybeAddNameHashToProfile(profile, item.first);
+    size_t name_hash_index =
+        MaybeAddNameHashToProfile(profile, item.first.name_hash);
 
     CallStackProfile::MetadataItem* profile_item = sample->add_metadata();
     profile_item->set_name_hash_index(name_hash_index);
+    if (item.first.key.has_value())
+      profile_item->set_key(*item.first.key);
     // Leave the value empty to indicate that the item was deleted.
   }
 

components/metrics/call_stack_profile_builder.h

@@ -13,6 +13,7 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/profiler/metadata_recorder.h"
 #include "base/profiler/profile_builder.h"
 #include "base/sampling_heap_profiler/module_cache.h"
@@ -99,6 +100,38 @@ class CallStackProfileBuilder : public base::ProfileBuilder {
                     const CallStackProfile::Stack* stack2) const;
   };
 
+  // Comparison function for the metadata map.
+  struct MetadataKey;
+  struct MetadataKeyCompare {
+    bool operator()(const MetadataKey& a, const MetadataKey& b) const;
+  };
+
+  // Definitions for a map-based representation of sample metadata.
+  struct MetadataKey {
+    MetadataKey(uint64_t name_hash, base::Optional<int64_t> key);
+
+    MetadataKey(const MetadataKey& other);
+    MetadataKey& operator=(const MetadataKey& other);
+
+    // The name_hash and optional user-specified key uniquely identifies a
+    // metadata value. See base::MetadataRecorder for details.
+    uint64_t name_hash;
+    base::Optional<int64_t> key;
+  };
+  using MetadataMap = std::map<MetadataKey, int64_t, MetadataKeyCompare>;
+
+  // Creates the metdata map from the array of items.
+  MetadataMap CreateMetadataMap(base::ProfileBuilder::MetadataItemArray items,
+                                size_t item_count);
+
+  // Returns all metadata items with new values in the current sample.
+  MetadataMap GetNewOrModifiedMetadataItems(const MetadataMap& current_items,
+                                            const MetadataMap& previous_items);
+
+  // Returns all metadata items deleted since the previous sample.
+  MetadataMap GetDeletedMetadataItems(const MetadataMap& current_items,
+                                      const MetadataMap& previous_items);
+
   // Adds the already-collected metadata to the sample.
   void AddSampleMetadata(CallStackProfile* profile,
                          CallStackProfile::StackSample* sample);
@@ -138,8 +171,9 @@ class CallStackProfileBuilder : public base::ProfileBuilder {
   // The data fetched from the MetadataRecorder for the next sample.
   base::ProfileBuilder::MetadataItemArray metadata_items_;
   size_t metadata_item_count_ = 0;
+
   // The data fetched from the MetadataRecorder for the previous sample.
-  std::map<uint64_t, int64_t> previous_items_;
+  MetadataMap previous_items_;
 
   // Maps metadata hash to index in |metadata_name_hash| array.
   std::unordered_map<uint64_t, int> metadata_hashes_cache_;