[tracing] Optimize TracedValue::AppendAsTraceFormat().

base/trace_event/trace_event_argument.cc

Index: base/trace_event/trace_event_argument.cc
diff --git a/base/trace_event/trace_event_argument.cc b/base/trace_event/trace_event_argument.cc
index 65965b1b286664c6d988e64bdef371cf2734c02d..c27a2f9ef3a6a6478199701ad5dfa2039ea82b49 100644
--- a/base/trace_event/trace_event_argument.cc
+++ b/base/trace_event/trace_event_argument.cc
@@ -6,11 +6,14 @@
 
 #include <stdint.h>
 
+#include <stack>
 #include <utility>
 
 #include "base/bits.h"
-#include "base/json/json_writer.h"
+#include "base/json/string_escape.h"
 #include "base/memory/ptr_util.h"
+#include "base/trace_event/common/trace_event_common.h"
+#include "base/trace_event/trace_event_impl.h"
 #include "base/trace_event/trace_event_memory_overhead.h"
 #include "base/values.h"
 
@@ -26,7 +29,7 @@ const char kTypeBool = 'b';
 const char kTypeInt = 'i';
 const char kTypeDouble = 'd';
 const char kTypeString = 's';
-const char kTypeCStr = '*';
+const char kTypeCStr = '*';  // only used for key names
 
 #ifndef NDEBUG
 const bool kStackTypeDict = false;
@@ -454,12 +457,100 @@ void TracedValue::AppendAsTraceFormat(std::string* out) const {
   DCHECK_CURRENT_CONTAINER_IS(kStackTypeDict);
   DCHECK_CONTAINER_STACK_DEPTH_EQ(1u);
 
-  // TODO(primiano): this could be smarter, skip the ToBaseValue encoding and
-  // produce the JSON on its own. This will require refactoring JSONWriter
-  // to decouple the base::Value traversal from the JSON writing bits
-  std::string tmp;
-  JSONWriter::Write(*ToBaseValue(), &tmp);
-  *out += tmp;
+  struct State {

[Primiano Tucci (use gerrit), 2017/07/12 15:26:23]

isn't this a gcc extension?

[DmitrySkiba, 2017/07/12 23:48:04]

Inner structs? I believe it's valid C++11.

+    enum Type { kTypeDict, kTypeArray };
+    Type type;
+    bool needs_comma;
+  };
+
+  auto maybe_append_key_name = [](State current_state, PickleIterator* it,
+                                  std::string* out) {
+    if (current_state.type == State::kTypeDict) {
+      EscapeJSONString(ReadKeyName(*it), true, out);
+      out->append(":");
+    }
+  };
+
+  std::stack<State> state_stack;
+
+  out->append("{");

[Primiano Tucci (use gerrit), 2017/07/12 15:26:23]

I think all ths might be even faster if you build the json using a stringstream
and only at the end assign the |out| string argument.
these std::string operations will very likely hit on the heap all the time

[DmitrySkiba, 2017/07/12 23:48:04]

I don't think there will be a difference - stringstream just appends to a
std::string inside. But after the dust settles, I do want to look closely at
std::string used as to accumulate JSONs.

+  state_stack.push({State::kTypeDict});
+
+  PickleIterator it(pickle_);
+  for (const char* type; it.ReadBytes(&type, 1);) {
+    switch (*type) {
+      case kTypeEndDict:
+        out->append("}");
+        state_stack.pop();
+        continue;
+
+      case kTypeEndArray:
+        out->append("]");
+        state_stack.pop();
+        continue;
+    }
+
+    State& current_state = state_stack.top();
+    if (current_state.needs_comma) {
+      out->append(",");
+    }
+
+    switch (*type) {
+      case kTypeStartDict:
+        maybe_append_key_name(current_state, &it, out);
+        out->append("{");
+        state_stack.push({State::kTypeDict});
+        break;
+
+      case kTypeStartArray:
+        maybe_append_key_name(current_state, &it, out);
+        out->append("[");
+        state_stack.push({State::kTypeArray});
+        break;
+
+      case kTypeBool: {
+        TraceEvent::TraceValue json_value;
+        CHECK(it.ReadBool(&json_value.as_bool));
+        maybe_append_key_name(current_state, &it, out);
+        TraceEvent::AppendValueAsJSON(TRACE_VALUE_TYPE_BOOL, json_value, out);
+      } break;
+
+      case kTypeInt: {
+        int value;
+        CHECK(it.ReadInt(&value));
+        maybe_append_key_name(current_state, &it, out);
+        TraceEvent::TraceValue json_value;
+        json_value.as_int = value;
+        TraceEvent::AppendValueAsJSON(TRACE_VALUE_TYPE_INT, json_value, out);
+      } break;
+
+      case kTypeDouble: {
+        TraceEvent::TraceValue json_value;
+        CHECK(it.ReadDouble(&json_value.as_double));
+        maybe_append_key_name(current_state, &it, out);
+        TraceEvent::AppendValueAsJSON(TRACE_VALUE_TYPE_DOUBLE, json_value, out);
+      } break;
+
+      case kTypeString: {
+        std::string value;
+        CHECK(it.ReadString(&value));
+        maybe_append_key_name(current_state, &it, out);
+        TraceEvent::TraceValue json_value;
+        json_value.as_string = value.c_str();
+        TraceEvent::AppendValueAsJSON(TRACE_VALUE_TYPE_STRING, json_value, out);
+      } break;
+
+      default:
+        NOTREACHED();
+    }
+
+    current_state.needs_comma = true;
+  }
+
+  out->append("}");
+  state_stack.pop();
+
+  DCHECK(state_stack.empty());
 }
 
 void TracedValue::EstimateTraceMemoryOverhead(