[tracing] Adding task information to heap profiler

base/trace_event/heap_profiler_type_name_deduplicator.cc

 // 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.
 
 #include "base/trace_event/heap_profiler_type_name_deduplicator.h"
 
 #include <stddef.h>
 #include <stdlib.h>
 #include <string>
 #include <utility>
 
+#include "base/files/file_path.h"
 #include "base/json/string_escape.h"
 #include "base/strings/stringprintf.h"
 #include "base/trace_event/trace_event_memory_overhead.h"
 
 namespace base {
 namespace trace_event {
 
+namespace {
+
+// Extract category name from the file name.
+std::string ExtractTypeInfo(const std::string& type_name) {

[Primiano Tucci (use gerrit), 2016/03/25 01:56:31]

I think the arg of this should be const char*, if you really need the char* to
string conversion you want to make it explicit, not hiding in the cast operator

Also I think this function could be simplified a bit as follows:
 - Convert the const char* into a StringPiece (which is a very cheap operation)
 - Use find_first_of / find_last_of (/,\)  to do the trimming on both side
 - Return a StringPiece and pass it to the json encoder.

Also I'd give this a more explicative (ExtractPostedTaskDirName) name and add a
comment to figure out what's going on.

[ssid, 2016/03/28 18:14:49]

Done.

+  std::vector<FilePath::StringType> folder_names;
+  FilePath(FILE_PATH_LITERAL(type_name)).DirName().GetComponents(&folder_names);
+
+  // If |type_name| is a not a file path the directory will be empty, then
+  // return the type name.
+  if (folder_names.empty())
+    return type_name;
+
+  // Remove the parent directory references - '..'.
+  while (folder_names.begin()->find(FilePath::kParentDirectory) !=
+         std::string::npos) {
+    folder_names.erase(folder_names.begin());
+  }
+  std::string type_info;
+  for (std::string folder : folder_names)

[Primiano Tucci (use gerrit), 2016/03/25 01:56:31]

tip for next times, const std::string& would have avoided a copy here.
But I think you can avoid any std::string at all in this function as suggested
above

[ssid, 2016/03/28 18:14:49]

Thanks.

+    type_info += folder + "/";
+  return type_info;
+}
+
+}  // namespace
+
 TypeNameDeduplicator::TypeNameDeduplicator() {
   // A null pointer has type ID 0 ("unknown type");
   type_ids_.insert(std::make_pair(nullptr, 0));
 }
 
 TypeNameDeduplicator::~TypeNameDeduplicator() {}
 
 int TypeNameDeduplicator::Insert(const char* type_name) {
   auto result = type_ids_.insert(std::make_pair(type_name, 0));
   auto& elem = result.first;
@@ skipping 17 matching lines @@
   // Write the first entry manually; the null pointer must not be dereferenced.
   // (The first entry is the null pointer because a |std::map| is ordered.)
   it++;
   out->append("\"0\":\"[unknown]\"");
 
   for (; it != type_ids_.end(); it++) {
     // Type IDs in the trace are strings, write them as stringified keys of
     // a dictionary.
     SStringPrintf(&buffer, ",\"%d\":", it->second);
 
+    // TODO(ssid): crbug.com/594803 the type name is misused for category name.
+    std::string type_info = ExtractTypeInfo(it->first);
+
     // |EscapeJSONString| appends, it does not overwrite |buffer|.
     bool put_in_quotes = true;
-    EscapeJSONString(it->first, put_in_quotes, &buffer);
+    EscapeJSONString(type_info, put_in_quotes, &buffer);
     out->append(buffer);
   }
 
   out->append("}");  // End the type names dictionary.
 }
 
 void TypeNameDeduplicator::EstimateTraceMemoryOverhead(
     TraceEventMemoryOverhead* overhead) {
   // The size here is only an estimate; it fails to take into account the size
   // of the tree nodes for the map, but as an estimate this should be fine.
   size_t map_size = type_ids_.size() * sizeof(std::pair<const char*, int>);
 
   overhead->Add("TypeNameDeduplicator",
                 sizeof(TypeNameDeduplicator) + map_size);
 }
 
 }  // namespace trace_event
 }  // namespace base