[tracing] Optimize TracedValue::AppendAsTraceFormat().

base/trace_event/trace_event_argument.cc

 // Copyright (c) 2014 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/trace_event_argument.h"
 
 #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"
 
 namespace base {
 namespace trace_event {
 
 namespace {
 const char kTypeStartDict = '{';
 const char kTypeEndDict = '}';
 const char kTypeStartArray = '[';
 const char kTypeEndArray = ']';
 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;
 const bool kStackTypeArray = true;
 #define DCHECK_CURRENT_CONTAINER_IS(x) DCHECK_EQ(x, nesting_stack_.back())
 #define DCHECK_CONTAINER_STACK_DEPTH_EQ(x) DCHECK_EQ(x, nesting_stack_.size())
 #define DEBUG_PUSH_CONTAINER(x) nesting_stack_.push_back(x)
 #define DEBUG_POP_CONTAINER() nesting_stack_.pop_back()
 #else
 #define DCHECK_CURRENT_CONTAINER_IS(x) do {} while (0)
@@ skipping 407 matching lines @@
     }
   }
   DCHECK(stack.empty());
   return std::move(root);
 }
 
 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(
     TraceEventMemoryOverhead* overhead) {
   overhead->Add(TraceEventMemoryOverhead::kTracedValue,
                 /* allocated size */
                 pickle_.GetTotalAllocatedSize(),
                 /* resident size */
                 pickle_.size());
 }
 
 }  // namespace trace_event
 }  // namespace base