Skip to content

Commit

Permalink
[tracing] Optimize TracedValue::AppendAsTraceFormat().
Browse files Browse the repository at this point in the history
This CL optimizes TracedValue::AppendAsTraceFormat() by removing
intermediate base::Value conversion.

HeapProfilerPerfTest.AppendStackFramesAsTraceFormat shows ~1.9x improvement
on macOS (1730ms -> 920ms).

TBR=jbauman@chromium.org

BUG=739378, 664350

Review-Url: https://codereview.chromium.org/2975033002
Cr-Commit-Position: refs/heads/master@{#487019}
  • Loading branch information
dskiba authored and Commit Bot committed Jul 16, 2017
1 parent 6da431c commit 131e21f
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 32 deletions.
107 changes: 99 additions & 8 deletions base/trace_event/trace_event_argument.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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;
Expand Down Expand Up @@ -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 {
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("{");
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(
Expand Down
28 changes: 14 additions & 14 deletions base/trace_event/trace_event_argument_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ namespace trace_event {

TEST(TraceEventArgumentTest, FlatDictionary) {
std::unique_ptr<TracedValue> value(new TracedValue());
value->SetInteger("int", 2014);
value->SetDouble("double", 0.0);
value->SetBoolean("bool", true);
value->SetDouble("double", 0.0);
value->SetInteger("int", 2014);
value->SetString("string", "string");
std::string json = "PREFIX";
value->AppendAsTraceFormat(&json);
Expand All @@ -30,9 +30,9 @@ TEST(TraceEventArgumentTest, FlatDictionary) {

TEST(TraceEventArgumentTest, NoDotPathExpansion) {
std::unique_ptr<TracedValue> value(new TracedValue());
value->SetInteger("in.t", 2014);
value->SetDouble("doub.le", 0.0);
value->SetBoolean("bo.ol", true);
value->SetDouble("doub.le", 0.0);
value->SetInteger("in.t", 2014);
value->SetString("str.ing", "str.ing");
std::string json;
value->AppendAsTraceFormat(&json);
Expand All @@ -43,23 +43,23 @@ TEST(TraceEventArgumentTest, NoDotPathExpansion) {

TEST(TraceEventArgumentTest, Hierarchy) {
std::unique_ptr<TracedValue> value(new TracedValue());
value->SetInteger("i0", 2014);
value->BeginDictionary("dict1");
value->SetInteger("i1", 2014);
value->BeginDictionary("dict2");
value->SetBoolean("b2", false);
value->EndDictionary();
value->SetString("s1", "foo");
value->EndDictionary();
value->SetDouble("d0", 0.0);
value->SetBoolean("b0", true);
value->BeginArray("a1");
value->AppendInteger(1);
value->AppendBoolean(true);
value->BeginDictionary();
value->SetInteger("i2", 3);
value->EndDictionary();
value->EndArray();
value->SetBoolean("b0", true);
value->SetDouble("d0", 0.0);
value->BeginDictionary("dict1");
value->BeginDictionary("dict2");
value->SetBoolean("b2", false);
value->EndDictionary();
value->SetInteger("i1", 2014);
value->SetString("s1", "foo");
value->EndDictionary();
value->SetInteger("i0", 2014);
value->SetString("s0", "foo");
std::string json;
value->AppendAsTraceFormat(&json);
Expand Down
4 changes: 2 additions & 2 deletions cc/base/filter_operations_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -970,8 +970,8 @@ TEST(FilterOperationsTest, ToString) {

filters.Append(FilterOperation::CreateSaturateFilter(3.f));
filters.Append(FilterOperation::CreateBlurFilter(2.f));
EXPECT_EQ(std::string("{\"FilterOperations\":[{\"amount\":3.0,\"type\":2},"
"{\"amount\":2.0,\"type\":8}]}"),
EXPECT_EQ(std::string("{\"FilterOperations\":[{\"type\":2,\"amount\":3.0},"
"{\"type\":8,\"amount\":2.0}]}"),
filters.ToString());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ TEST(GraphicsMemoryDumpProviderTest, ParseResponse) {
std::string json;
mad->attributes_for_testing()->AppendAsTraceFormat(&json);
ASSERT_EQ(
"{\"memtrack_pss\":{\"type\":\"scalar\",\"units\":\"bytes\",\"value\":"
"\"22\"},"
"\"memtrack_total\":{\"type\":\"scalar\",\"units\":\"bytes\",\"value\":"
"\"c\"}}",
"{\"memtrack_total\":{\"type\":\"scalar\",\"units\":\"bytes\",\"value\":"
"\"c\"},"
"\"memtrack_pss\":{\"type\":\"scalar\",\"units\":\"bytes\",\"value\":"
"\"22\"}}",
json);

// Check the "gl" row.
Expand All @@ -37,10 +37,10 @@ TEST(GraphicsMemoryDumpProviderTest, ParseResponse) {
json = "";
mad->attributes_for_testing()->AppendAsTraceFormat(&json);
ASSERT_EQ(
"{\"memtrack_pss\":{\"type\":\"scalar\",\"units\":\"bytes\",\"value\":"
"\"4e\"},"
"\"memtrack_total\":{\"type\":\"scalar\",\"units\":\"bytes\",\"value\":"
"\"38\"}}",
"{\"memtrack_total\":{\"type\":\"scalar\",\"units\":\"bytes\",\"value\":"
"\"38\"},"
"\"memtrack_pss\":{\"type\":\"scalar\",\"units\":\"bytes\",\"value\":"
"\"4e\"}}",
json);

// Test for truncated input.
Expand Down

0 comments on commit 131e21f

Please sign in to comment.