Revert "Perfetto: Optimization to avoid string table lookup for _END events"

This reverts commit a7afe430.

Reason for revert: I suspect this may have caused crashes on https://ci.chromium.org/p/chrome/builders/luci.chrome.ci/linux-perf-fyi#

Original change's description:
> Perfetto: Optimization to avoid string table lookup for _END events
> 
> If we're emitting an _END trace event, we know that the string
> table entries for the name and the categories have already been
> emitted and so we don't need to check the string table in this
> case.
> 
> R=​primiano@chromium.org,skyostil@chromium.org
> 
> Change-Id: I9107bffb0b48f7b71d0a42961a78a3861ffcb76f
> Reviewed-on: https://chromium-review.googlesource.com/c/1258043
> Commit-Queue: oysteine <oysteine@chromium.org>
> Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#600183}

TBR=primiano@chromium.org,oysteine@chromium.org,skyostil@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: Ia460750fcaf338c4bf24ee7317d58b7aeede9520
Reviewed-on: https://chromium-review.googlesource.com/c/1293612Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601586}

services/tracing/public/cpp/perfetto/trace_event_data_source.cc

@@ -7,7 +7,6 @@
 #include <map>
 #include <utility>
 
-#include "base/hash.h"
 #include "base/json/json_writer.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/no_destructor.h"
@@ -128,35 +127,28 @@ class TraceEventDataSource::ThreadLocalEventSink {
     trace_packet_handle_ = trace_writer_->NewTracePacket();
     event_bundle_ =
         ChromeEventBundleHandle(trace_packet_handle_->set_chrome_events());
-    interned_strings_.clear();
-#if DCHECK_IS_ON()
-    hash_verifier_.clear();
-#endif
+    string_table_.clear();
+    next_string_table_index_ = 0;
   }
 
-  uint32_t GetStringTableIndexForString(const char* str_value) {
+  int GetStringTableIndexForString(const char* str_value) {
     EnsureValidHandles();
 
-    uint32_t string_table_index = base::Hash(str_value, strlen(str_value));
+    auto it = string_table_.find(reinterpret_cast<intptr_t>(str_value));
+    if (it != string_table_.end()) {
+      CHECK_EQ(std::string(reinterpret_cast<const char*>(it->first)),
+               std::string(str_value));
 
-    if (interned_strings_.find(string_table_index) != interned_strings_.end()) {
-#if DCHECK_IS_ON()
-      DCHECK_EQ(hash_verifier_[string_table_index], str_value);
-#endif
-
-      return string_table_index;
+      return it->second;
     }
 
-    interned_strings_.insert(string_table_index);
+    int string_table_index = ++next_string_table_index_;
+    string_table_[reinterpret_cast<intptr_t>(str_value)] = string_table_index;
 
     auto* new_string_table_entry = event_bundle_->add_string_table();
     new_string_table_entry->set_value(str_value);
     new_string_table_entry->set_index(string_table_index);
 
-#if DCHECK_IS_ON()
-    hash_verifier_[string_table_index] = str_value;
-#endif
-
     return string_table_index;
   }
 
@@ -184,11 +176,9 @@ class TraceEventDataSource::ThreadLocalEventSink {
 
     EnsureValidHandles();
 
-    uint32_t name_index = 0;
-    uint32_t category_name_index = 0;
-    uint32_t arg_name_indices[base::trace_event::kTraceMaxNumArgs] = {0};
-
-    char phase = trace_event.phase();
+    int name_index = 0;
+    int category_name_index = 0;
+    int arg_name_indices[base::trace_event::kTraceMaxNumArgs] = {0};
 
     // Populate any new string table parts first; has to be done before
     // the add_trace_events() call (as the string table is part of the outer
@@ -198,18 +188,9 @@ class TraceEventDataSource::ThreadLocalEventSink {
     // the string every time.
     bool string_table_enabled = !(trace_event.flags() & TRACE_EVENT_FLAG_COPY);
     if (string_table_enabled) {
-      // Optimization: If it's an _END event, we know that the string table
-      // entries for the name and the category have already been emitted.
-      if (phase == TRACE_EVENT_PHASE_END) {
-        name_index = base::Hash(trace_event.name());
-        category_name_index = base::Hash(TraceLog::GetCategoryGroupName(
-            trace_event.category_group_enabled()));
-      } else {
-        name_index = GetStringTableIndexForString(trace_event.name());
-        category_name_index =
-            GetStringTableIndexForString(TraceLog::GetCategoryGroupName(
-                trace_event.category_group_enabled()));
-      }
+      name_index = GetStringTableIndexForString(trace_event.name());
+      category_name_index = GetStringTableIndexForString(
+          TraceLog::GetCategoryGroupName(trace_event.category_group_enabled()));
 
       for (int i = 0;
            i < base::trace_event::kTraceMaxNumArgs && trace_event.arg_name(i);
@@ -254,6 +235,7 @@ class TraceEventDataSource::ThreadLocalEventSink {
     new_trace_event->set_process_id(process_id);
     new_trace_event->set_thread_id(thread_id);
 
+    char phase = trace_event.phase();
     new_trace_event->set_phase(phase);
 
     for (int i = 0;
@@ -306,6 +288,11 @@ class TraceEventDataSource::ThreadLocalEventSink {
       int64_t duration = trace_event.duration().InMicroseconds();
       if (duration != -1) {
         new_trace_event->set_duration(duration);
+      } else {
+        // TODO(oysteine): Workaround until TRACE_EVENT_PHASE_COMPLETE can be
+        // split into begin/end pairs. If the duration is -1 and the
+        // trace-viewer will spend forever generating a warning for each event.
+        new_trace_event->set_duration(0);
       }
 
       if (!trace_event.thread_timestamp().is_null()) {
@@ -342,16 +329,16 @@ class TraceEventDataSource::ThreadLocalEventSink {
     event_bundle_ = ChromeEventBundleHandle();
     trace_packet_handle_ = perfetto::TraceWriter::TracePacketHandle();
     trace_writer_->Flush();
+    token_ = "";
   }
 
  private:
   std::unique_ptr<perfetto::TraceWriter> trace_writer_;
   ChromeEventBundleHandle event_bundle_;
   perfetto::TraceWriter::TracePacketHandle trace_packet_handle_;
-  std::set<uint32_t> interned_strings_;
-#if DCHECK_IS_ON()
-  std::map<uint32_t, std::string> hash_verifier_;
-#endif
+  std::map<intptr_t, int> string_table_;
+  int next_string_table_index_ = 0;
+  std::string token_;
 };
 
 namespace {