Merge "trace_processor: fix multiple bugs with filtering and casting"
diff --git a/src/profiling/memory/README.md b/src/profiling/memory/README.md
index a9a5b9c..602fa1d 100644
--- a/src/profiling/memory/README.md
+++ b/src/profiling/memory/README.md
@@ -14,24 +14,33 @@
```bash
INTERVAL=128000
-adb root
-adb shell setenforce 0
-adb shell heapprofd -s -i ${INTERVAL} &
-adb shell kill -36 ${PID} # Start profiling the process.
+adb shell su root setenforce 0
+adb shell su root start heapprofd
+
+echo '
+buffers {
+ size_kb: 100024
+}
+
+data_sources {
+ config {
+ name: "android.heapprofd"
+ target_buffer: 0
+ heapprofd_config {
+ sampling_interval_bytes: '${INTERVAL}'
+ pid: '${PID}'
+ continuous_dump_config {
+ dump_phase_ms: 10000
+ dump_interval_ms: 1000
+ }
+ }
+ }
+}
+
+duration_ms: 20000
+' | adb shell perfetto -t -c - -o /data/misc/perfetto-traces/trace
+
+adb pull /data/misc/perfetto-traces/trace /tmp/trace
```
-To obtain heap dumps for all profiled processes, send `SIGUSR1` to heapprofd
-which produces heap dumps in /data/local/tmp.
-
-```bash
-adb shell killall -USR1 heapprofd
-adb pull /data/local/tmp/heap_dump.${PID}
-```
-
-This file can then be converted to a flamegraph using Brendan Gregg's
-[`flamegraph.pl`](
- https://github.com/brendangregg/FlameGraph/blob/master/flamegraph.pl).
-
-```bash
-flamegraph.pl heap_dump.${PID} > heap_dump.${PID}.svg
-```
+TODO(fmayer): Add instructions how to visualize the trace.
diff --git a/src/trace_processor/BUILD.gn b/src/trace_processor/BUILD.gn
index 7f5fbca..132565b 100644
--- a/src/trace_processor/BUILD.gn
+++ b/src/trace_processor/BUILD.gn
@@ -176,6 +176,10 @@
"../../protos/perfetto/trace:lite",
"../base",
]
+ if (perfetto_build_standalone) {
+ sources += [ "json_trace_parser_unittest.cc" ]
+ deps += [ "../../gn:jsoncpp_deps" ]
+ }
}
source_set("integrationtests") {
diff --git a/src/trace_processor/json_trace_parser.cc b/src/trace_processor/json_trace_parser.cc
index 245348c..5bb3b28 100644
--- a/src/trace_processor/json_trace_parser.cc
+++ b/src/trace_processor/json_trace_parser.cc
@@ -16,6 +16,7 @@
#include "src/trace_processor/json_trace_parser.h"
+#include <inttypes.h>
#include <json/reader.h>
#include <json/value.h>
@@ -36,7 +37,6 @@
namespace perfetto {
namespace trace_processor {
-
namespace {
enum ReadDictRes { kFoundDict, kNeedsMoreData, kEndOfTrace, kFatalError };
@@ -82,6 +82,31 @@
} // namespace
+bool CoerceToUint64(const Json::Value& value, uint64_t* integer_ptr) {
+ switch (static_cast<size_t>(value.type())) {
+ case Json::uintValue:
+ case Json::intValue:
+ *integer_ptr = value.asUInt64();
+ return true;
+ case Json::stringValue: {
+ std::string s = value.asString();
+ char* end;
+ *integer_ptr = strtoull(s.c_str(), &end, 10);
+ return end == s.data() + s.size();
+ }
+ default:
+ return false;
+ }
+}
+
+bool CoerceToUint32(const Json::Value& value, uint32_t* integer_ptr) {
+ uint64_t n = 0;
+ if (!CoerceToUint64(value, &n))
+ return false;
+ *integer_ptr = static_cast<uint32_t>(n);
+ return true;
+}
+
JsonTraceParser::JsonTraceParser(TraceProcessorContext* context)
: context_(context) {}
@@ -124,9 +149,13 @@
if (!ph.isString())
continue;
char phase = *ph.asCString();
- uint32_t tid = value["tid"].asUInt();
- uint32_t pid = value["pid"].asUInt();
- uint64_t ts = value["ts"].asLargestUInt() * 1000;
+ uint32_t tid = 0;
+ PERFETTO_CHECK(CoerceToUint32(value["tid"], &tid));
+ uint32_t pid = 0;
+ PERFETTO_CHECK(CoerceToUint32(value["pid"], &pid));
+ uint64_t ts = 0;
+ PERFETTO_CHECK(CoerceToUint64(value["ts"], &ts));
+ ts *= 1000;
const char* cat = value.isMember("cat") ? value["cat"].asCString() : "";
const char* name = value.isMember("name") ? value["name"].asCString() : "";
StringId cat_id = storage->InternString(cat);
@@ -143,7 +172,9 @@
break;
}
case 'X': { // TRACE_EVENT (scoped event).
- uint64_t duration = value["dur"].asUInt() * 1000;
+ uint64_t duration = 0;
+ PERFETTO_CHECK(CoerceToUint64(value["ts"], &duration));
+ duration *= 1000;
slice_tracker->Scoped(ts, utid, cat_id, name_id, duration);
break;
}
diff --git a/src/trace_processor/json_trace_parser.h b/src/trace_processor/json_trace_parser.h
index bdd9378..36f001d 100644
--- a/src/trace_processor/json_trace_parser.h
+++ b/src/trace_processor/json_trace_parser.h
@@ -26,11 +26,19 @@
#include "src/trace_processor/chunked_trace_reader.h"
#include "src/trace_processor/trace_storage.h"
+namespace Json {
+class Value;
+}
+
namespace perfetto {
namespace trace_processor {
class TraceProcessorContext;
+// TODO(hjd): Make optional.
+bool CoerceToUint64(const Json::Value& value, uint64_t* integer_ptr);
+bool CoerceToUint32(const Json::Value& value, uint32_t* integer_ptr);
+
// Parses legacy chrome JSON traces. The support for now is extremely rough
// and supports only explicit TRACE_EVENT_BEGIN/END events.
class JsonTraceParser : public ChunkedTraceReader {
diff --git a/src/trace_processor/json_trace_parser_unittest.cc b/src/trace_processor/json_trace_parser_unittest.cc
new file mode 100644
index 0000000..be23d7a
--- /dev/null
+++ b/src/trace_processor/json_trace_parser_unittest.cc
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "src/trace_processor/json_trace_parser.h"
+
+#include <json/value.h>
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace perfetto {
+namespace trace_processor {
+namespace {
+
+TEST(JsonTraceParserTest, CoerceToUint32) {
+ uint32_t n = 0;
+
+ ASSERT_TRUE(CoerceToUint32(Json::Value(42), &n));
+ EXPECT_EQ(n, 42);
+
+ ASSERT_TRUE(CoerceToUint32(Json::Value("42"), &n));
+ EXPECT_EQ(n, 42);
+}
+
+TEST(JsonTraceParserTest, CoerceToUint64) {
+ uint64_t n = 0;
+
+ ASSERT_TRUE(CoerceToUint64(Json::Value(42), &n));
+ EXPECT_EQ(n, 42);
+
+ ASSERT_TRUE(CoerceToUint64(Json::Value("42"), &n));
+ EXPECT_EQ(n, 42);
+
+ ASSERT_FALSE(CoerceToUint64(Json::Value("foo"), &n));
+ ASSERT_FALSE(CoerceToUint64(Json::Value("1234!"), &n));
+}
+
+} // namespace
+} // namespace trace_processor
+} // namespace perfetto
diff --git a/src/trace_processor/trace_processor_impl_unittest.cc b/src/trace_processor/trace_processor_impl_unittest.cc
index a901de6..9200d4e 100644
--- a/src/trace_processor/trace_processor_impl_unittest.cc
+++ b/src/trace_processor/trace_processor_impl_unittest.cc
@@ -23,27 +23,27 @@
namespace trace_processor {
namespace {
-TEST(TraceProcessorImpl, GuessTraceType_Empty) {
+TEST(TraceProcessorImplTest, GuessTraceType_Empty) {
const uint8_t prefix[] = "";
EXPECT_EQ(kUnknownTraceType, GuessTraceType(prefix, 0));
}
-TEST(TraceProcessorImpl, GuessTraceType_Json) {
+TEST(TraceProcessorImplTest, GuessTraceType_Json) {
const uint8_t prefix[] = "{\"traceEvents\":[";
EXPECT_EQ(kJsonTraceType, GuessTraceType(prefix, sizeof(prefix)));
}
-TEST(TraceProcessorImpl, GuessTraceType_JsonWithSpaces) {
+TEST(TraceProcessorImplTest, GuessTraceType_JsonWithSpaces) {
const uint8_t prefix[] = "\n{ \"traceEvents\": [";
EXPECT_EQ(kJsonTraceType, GuessTraceType(prefix, sizeof(prefix)));
}
-TEST(TraceProcessorImpl, GuessTraceType_JsonMissingTraceEvents) {
+TEST(TraceProcessorImplTest, GuessTraceType_JsonMissingTraceEvents) {
const uint8_t prefix[] = "[{";
EXPECT_EQ(kJsonTraceType, GuessTraceType(prefix, sizeof(prefix)));
}
-TEST(TraceProcessorImpl, GuessTraceType_Proto) {
+TEST(TraceProcessorImplTest, GuessTraceType_Proto) {
const uint8_t prefix[] = {0x0a, 0x65, 0x18, 0x8f, 0x4e, 0x32, 0x60, 0x0a};
EXPECT_EQ(kProtoTraceType, GuessTraceType(prefix, sizeof(prefix)));
}
diff --git a/src/traced/probes/ftrace/atrace_wrapper.cc b/src/traced/probes/ftrace/atrace_wrapper.cc
index db50e09..5987d46 100644
--- a/src/traced/probes/ftrace/atrace_wrapper.cc
+++ b/src/traced/probes/ftrace/atrace_wrapper.cc
@@ -44,19 +44,61 @@
argv.push_back(const_cast<char*>(arg.c_str()));
argv.push_back(nullptr);
+ // Create the pipe for the child process to return stderr.
+ int filedes[2];
+ if (pipe(filedes) == -1)
+ return false;
+
pid_t pid = fork();
PERFETTO_CHECK(pid >= 0);
if (pid == 0) {
- // Close stdin/out/err + any file descriptor that we might have mistakenly
+ // Duplicate the write end of the pipe into stderr.
+ if ((dup2(filedes[1], STDERR_FILENO) == -1)) {
+ const char kError[] = "Unable to duplicate stderr fd";
+ write(filedes[1], kError, sizeof(kError));
+ _exit(1);
+ }
+
+ // Close stdin/out + any file descriptor that we might have mistakenly
// not marked as FD_CLOEXEC.
- for (int i = 0; i < 128; i++)
- close(i);
+ for (int i = 0; i < 128; i++) {
+ if (i != STDERR_FILENO)
+ close(i);
+ }
+
+ // Close the read and write end of the pipe fds.
+ close(filedes[1]);
+ close(filedes[0]);
+
execv("/system/bin/atrace", &argv[0]);
// Reached only if execv fails.
_exit(1);
}
+
+ // Close the write end of the pipe.
+ close(filedes[1]);
+
+ // Collect the output from child process.
+ std::string error;
+ char buffer[4096];
+ while (true) {
+ ssize_t count = PERFETTO_EINTR(read(filedes[0], buffer, sizeof(buffer)));
+ if (count == 0 || count == -1)
+ break;
+ error.append(buffer, static_cast<size_t>(count));
+ }
+ // Close the read end of the pipe.
+ close(filedes[0]);
+
+ // Wait until the child process exits fully.
PERFETTO_EINTR(waitpid(pid, &status, 0));
- return status == 0;
+
+ bool ok = WIFEXITED(status) && WEXITSTATUS(status) == 0;
+ if (!ok) {
+ // TODO(lalitm): use the stderr result from atrace.
+ base::ignore_result(error);
+ }
+ return ok;
}
#endif
diff --git a/ui/src/assets/topbar.scss b/ui/src/assets/topbar.scss
index 81a4a1b..62adf06 100644
--- a/ui/src/assets/topbar.scss
+++ b/ui/src/assets/topbar.scss
@@ -56,9 +56,9 @@
outline: none;
}
&::placeholder {
- color: #aaa;
+ color: #b4b7ba;
font-family: 'Raleway';
- font-weight: 100;
+ font-weight: 400;
}
}
&.command-mode {
