Create proper table for packages list.
Bug: 150546312
Change-Id: Ib22ac967c075db95d661f44c2291554a3befccea
diff --git a/Android.bp b/Android.bp
index 53c1ddc..e3d4697 100644
--- a/Android.bp
+++ b/Android.bp
@@ -6527,6 +6527,7 @@
"src/trace_processor/importers/json/json_tracker.cc",
"src/trace_processor/importers/proto/android_probes_module.cc",
"src/trace_processor/importers/proto/android_probes_parser.cc",
+ "src/trace_processor/importers/proto/android_probes_tracker.cc",
"src/trace_processor/importers/proto/graphics_event_module.cc",
"src/trace_processor/importers/proto/graphics_event_parser.cc",
"src/trace_processor/importers/proto/heap_graph_module.cc",
diff --git a/BUILD b/BUILD
index 922f2f0..a50a6fd 100644
--- a/BUILD
+++ b/BUILD
@@ -923,6 +923,8 @@
"src/trace_processor/importers/proto/android_probes_module.h",
"src/trace_processor/importers/proto/android_probes_parser.cc",
"src/trace_processor/importers/proto/android_probes_parser.h",
+ "src/trace_processor/importers/proto/android_probes_tracker.cc",
+ "src/trace_processor/importers/proto/android_probes_tracker.h",
"src/trace_processor/importers/proto/graphics_event_module.cc",
"src/trace_processor/importers/proto/graphics_event_module.h",
"src/trace_processor/importers/proto/graphics_event_parser.cc",
diff --git a/src/trace_processor/BUILD.gn b/src/trace_processor/BUILD.gn
index 51f93d7..6138ab6 100644
--- a/src/trace_processor/BUILD.gn
+++ b/src/trace_processor/BUILD.gn
@@ -226,6 +226,8 @@
"importers/proto/android_probes_module.h",
"importers/proto/android_probes_parser.cc",
"importers/proto/android_probes_parser.h",
+ "importers/proto/android_probes_tracker.cc",
+ "importers/proto/android_probes_tracker.h",
"importers/proto/graphics_event_module.cc",
"importers/proto/graphics_event_module.h",
"importers/proto/graphics_event_parser.cc",
diff --git a/src/trace_processor/importers/proto/android_probes_parser.cc b/src/trace_processor/importers/proto/android_probes_parser.cc
index 1952fef..1f19644 100644
--- a/src/trace_processor/importers/proto/android_probes_parser.cc
+++ b/src/trace_processor/importers/proto/android_probes_parser.cc
@@ -38,6 +38,8 @@
#include "protos/perfetto/trace/sys_stats/sys_stats.pbzero.h"
#include "protos/perfetto/trace/system_info.pbzero.h"
+#include "src/trace_processor/importers/proto/android_probes_tracker.h"
+
namespace perfetto {
namespace trace_processor {
@@ -221,26 +223,19 @@
context_->storage->SetStats(stats::packages_list_has_parse_errors,
pkg_list.parse_error());
- // Insert the package info into arg sets (one set per package), with the arg
- // set ids collected in the Metadata table, under
- // metadata::android_packages_list key type.
+ AndroidProbesTracker* tracker = AndroidProbesTracker::GetOrCreate(context_);
for (auto it = pkg_list.packages(); it; ++it) {
- // Insert a placeholder metadata entry, which will be overwritten by the
- // arg_set_id when the arg tracker is flushed.
- auto id = context_->metadata_tracker->AppendMetadata(
- metadata::android_packages_list, Variadic::Integer(0));
- auto add_arg = [this, id](base::StringView name, Variadic value) {
- StringId key_id = context_->storage->InternString(name);
- context_->args_tracker->AddArgsTo(id).AddArg(key_id, value);
- };
protos::pbzero::PackagesList_PackageInfo::Decoder pkg(*it);
- add_arg("name",
- Variadic::String(context_->storage->InternString(pkg.name())));
- add_arg("uid", Variadic::UnsignedInteger(pkg.uid()));
- add_arg("debuggable", Variadic::Boolean(pkg.debuggable()));
- add_arg("profileable_from_shell",
- Variadic::Boolean(pkg.profileable_from_shell()));
- add_arg("version_code", Variadic::Integer(pkg.version_code()));
+ std::string pkg_name = pkg.name().ToStdString();
+ if (!tracker->ShouldInsertPackage(pkg_name)) {
+ continue;
+ }
+ context_->storage->mutable_package_list_table()->Insert(
+ {context_->storage->InternString(pkg.name()),
+ static_cast<int64_t>(pkg.uid()), pkg.debuggable(),
+ pkg.profileable_from_shell(),
+ static_cast<int64_t>(pkg.version_code())});
+ tracker->InsertedPackage(std::move(pkg_name));
}
}
diff --git a/src/trace_processor/importers/proto/android_probes_tracker.cc b/src/trace_processor/importers/proto/android_probes_tracker.cc
new file mode 100644
index 0000000..490ac87
--- /dev/null
+++ b/src/trace_processor/importers/proto/android_probes_tracker.cc
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2020 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/importers/proto/android_probes_tracker.h"
+
+namespace perfetto {
+namespace trace_processor {
+
+AndroidProbesTracker::AndroidProbesTracker(TraceProcessorContext*) {}
+
+AndroidProbesTracker::~AndroidProbesTracker() = default;
+
+} // namespace trace_processor
+} // namespace perfetto
diff --git a/src/trace_processor/importers/proto/android_probes_tracker.h b/src/trace_processor/importers/proto/android_probes_tracker.h
new file mode 100644
index 0000000..f50fd81
--- /dev/null
+++ b/src/trace_processor/importers/proto/android_probes_tracker.h
@@ -0,0 +1,61 @@
+/*
+ * Copyright (C) 2020 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.
+ */
+
+#ifndef SRC_TRACE_PROCESSOR_IMPORTERS_PROTO_ANDROID_PROBES_TRACKER_H_
+#define SRC_TRACE_PROCESSOR_IMPORTERS_PROTO_ANDROID_PROBES_TRACKER_H_
+
+#include <set>
+
+#include "perfetto/ext/base/optional.h"
+
+#include "src/trace_processor/storage/trace_storage.h"
+#include "src/trace_processor/trace_processor_context.h"
+
+namespace perfetto {
+namespace trace_processor {
+
+class TraceProcessorContext;
+
+class AndroidProbesTracker : public Destructible {
+ public:
+ explicit AndroidProbesTracker(TraceProcessorContext*);
+ ~AndroidProbesTracker() override;
+
+ static AndroidProbesTracker* GetOrCreate(TraceProcessorContext* context) {
+ if (!context->android_probes_tracker) {
+ context->android_probes_tracker.reset(new AndroidProbesTracker(context));
+ }
+ return static_cast<AndroidProbesTracker*>(
+ context->android_probes_tracker.get());
+ }
+
+ bool ShouldInsertPackage(const std::string& package_name) const {
+ auto it = seen_packages_.find(package_name);
+ return it == seen_packages_.end();
+ }
+
+ void InsertedPackage(std::string package_name) {
+ seen_packages_.emplace(std::move(package_name));
+ }
+
+ private:
+ std::set<std::string> seen_packages_;
+};
+
+} // namespace trace_processor
+} // namespace perfetto
+
+#endif // SRC_TRACE_PROCESSOR_IMPORTERS_PROTO_ANDROID_PROBES_TRACKER_H_
diff --git a/src/trace_processor/importers/proto/proto_trace_parser_unittest.cc b/src/trace_processor/importers/proto/proto_trace_parser_unittest.cc
index 338e58d..e166e45 100644
--- a/src/trace_processor/importers/proto/proto_trace_parser_unittest.cc
+++ b/src/trace_processor/importers/proto/proto_trace_parser_unittest.cc
@@ -2462,41 +2462,67 @@
// The relevant arg sets have the info about the packages. To simplify test
// structure, make an assumption that metadata storage is filled in in the
// FIFO order of seen packages.
- const auto& args = context_.storage->arg_table();
- const auto& metadata = context_.storage->metadata_table();
-
- Table package_list = metadata.Filter(
- {metadata.name().eq(metadata::kNames[metadata::android_packages_list])});
+ const auto& package_list = context_.storage->package_list_table();
ASSERT_EQ(package_list.row_count(), 2u);
- const Column* int_value = package_list.GetColumnByName("int_value");
- uint32_t first_set_id = static_cast<uint32_t>(int_value->Get(0).long_value);
- uint32_t second_set_id = static_cast<uint32_t>(int_value->Get(1).long_value);
+ EXPECT_STREQ(storage_->GetString(package_list.package_name()[0]).c_str(),
+ "com.test.app");
+ EXPECT_EQ(package_list.uid()[0], 1000u);
+ EXPECT_EQ(package_list.debuggable()[0], false);
+ EXPECT_EQ(package_list.profileable_from_shell()[0], true);
+ EXPECT_EQ(package_list.version_code()[0], 42);
- // helper to look up arg values
- auto find_arg = [&args, this](ArgSetId set_id, const char* arg_name) {
- for (uint32_t i = 0; i < args.row_count(); i++) {
- if (args.arg_set_id()[i] == set_id &&
- args.key()[i] == storage_->InternString(arg_name))
- return storage_->GetArgValue(i);
- }
- PERFETTO_FATAL("Didn't find expected argument");
- };
+ EXPECT_STREQ(storage_->GetString(package_list.package_name()[1]).c_str(),
+ "com.test.app2");
+ EXPECT_EQ(package_list.uid()[1], 1001u);
+ EXPECT_EQ(package_list.debuggable()[1], false);
+ EXPECT_EQ(package_list.profileable_from_shell()[1], false);
+ EXPECT_EQ(package_list.version_code()[1], 43);
+}
- auto first_name_id = find_arg(first_set_id, "name").string_value;
- EXPECT_STREQ(storage_->GetString(first_name_id).c_str(), "com.test.app");
- EXPECT_EQ(find_arg(first_set_id, "uid").uint_value, 1000u);
- EXPECT_EQ(find_arg(first_set_id, "debuggable").bool_value, false);
- EXPECT_EQ(find_arg(first_set_id, "profileable_from_shell").bool_value, true);
- EXPECT_EQ(find_arg(first_set_id, "version_code").int_value, 42);
+TEST_F(ProtoTraceParserTest, AndroidPackagesListDuplicate) {
+ auto* packet = trace_.add_packet();
+ auto* pkg_list = packet->set_packages_list();
- auto second_name_id = find_arg(second_set_id, "name").string_value;
- EXPECT_STREQ(storage_->GetString(second_name_id).c_str(), "com.test.app2");
- EXPECT_EQ(find_arg(second_set_id, "uid").uint_value, 1001u);
- EXPECT_EQ(find_arg(second_set_id, "debuggable").bool_value, false);
- EXPECT_EQ(find_arg(second_set_id, "profileable_from_shell").bool_value,
- false);
- EXPECT_EQ(find_arg(second_set_id, "version_code").int_value, 43);
+ pkg_list->set_read_error(false);
+ pkg_list->set_parse_error(true);
+ {
+ auto* pkg = pkg_list->add_packages();
+ pkg->set_name("com.test.app");
+ pkg->set_uid(1000);
+ pkg->set_debuggable(false);
+ pkg->set_profileable_from_shell(true);
+ pkg->set_version_code(42);
+ }
+ {
+ auto* pkg = pkg_list->add_packages();
+ pkg->set_name("com.test.app");
+ pkg->set_uid(1000);
+ pkg->set_debuggable(false);
+ pkg->set_profileable_from_shell(true);
+ pkg->set_version_code(42);
+ }
+
+ Tokenize();
+
+ // Packet-level errors reflected in stats storage.
+ const auto& stats = context_.storage->stats();
+ EXPECT_FALSE(stats[stats::packages_list_has_read_errors].value);
+ EXPECT_TRUE(stats[stats::packages_list_has_parse_errors].value);
+
+ // Expect two metadata rows, each with an int_value of a separate arg set id.
+ // The relevant arg sets have the info about the packages. To simplify test
+ // structure, make an assumption that metadata storage is filled in in the
+ // FIFO order of seen packages.
+ const auto& package_list = context_.storage->package_list_table();
+ ASSERT_EQ(package_list.row_count(), 1u);
+
+ EXPECT_STREQ(storage_->GetString(package_list.package_name()[0]).c_str(),
+ "com.test.app");
+ EXPECT_EQ(package_list.uid()[0], 1000u);
+ EXPECT_EQ(package_list.debuggable()[0], false);
+ EXPECT_EQ(package_list.profileable_from_shell()[0], true);
+ EXPECT_EQ(package_list.version_code()[0], 42);
}
TEST_F(ProtoTraceParserTest, ParseCPUProfileSamplesIntoTable) {
diff --git a/src/trace_processor/metrics/android/android_package_list.sql b/src/trace_processor/metrics/android/android_package_list.sql
index 7ca9c15..5e05281 100644
--- a/src/trace_processor/metrics/android/android_package_list.sql
+++ b/src/trace_processor/metrics/android/android_package_list.sql
@@ -14,43 +14,6 @@
-- limitations under the License.
--
--- Get distinct packages list
-DROP VIEW IF EXISTS package_arg_ids;
-
-CREATE VIEW package_arg_ids AS
-SELECT int_value AS arg_set_id
-FROM metadata WHERE name = 'android_packages_list';
-
--- Generate a table mapping package names to their attributes
-DROP VIEW IF EXISTS package_args;
-
-CREATE VIEW package_args AS
-SELECT arg_set_id, key, string_value, int_value
-FROM package_arg_ids JOIN args USING(arg_set_id);
-
-DROP TABLE IF EXISTS package_list;
-
-CREATE TABLE package_list(
- package_name TEXT PRIMARY KEY,
- uid INT,
- version_code INT,
- debuggable INT
-);
-
-INSERT OR REPLACE INTO package_list
-SELECT names.name, uids.uid, versions.version, debuggable.is_debug
-FROM
- (SELECT arg_set_id, string_value name FROM package_args WHERE key = 'name')
- AS names
- JOIN (SELECT arg_set_id, int_value uid FROM package_args WHERE key = 'uid')
- AS uids USING (arg_set_id)
- JOIN (SELECT arg_set_id, int_value version FROM package_args WHERE key = 'version_code')
- AS versions USING (arg_set_id)
- JOIN (SELECT arg_set_id, int_value is_debug FROM package_args WHERE key = 'debuggable')
- AS debuggable USING (arg_set_id);
-
-DROP VIEW IF EXISTS android_package_list_output;
-
CREATE VIEW android_package_list_output AS
SELECT AndroidPackageList(
'packages', (
diff --git a/src/trace_processor/metrics/android/process_metadata.sql b/src/trace_processor/metrics/android/process_metadata.sql
index d38795f..401948d 100644
--- a/src/trace_processor/metrics/android/process_metadata.sql
+++ b/src/trace_processor/metrics/android/process_metadata.sql
@@ -14,8 +14,6 @@
-- limitations under the License.
--
-SELECT RUN_METRIC('android/android_package_list.sql');
-
DROP TABLE IF EXISTS uid_package_count;
CREATE TABLE uid_package_count AS
diff --git a/src/trace_processor/storage/metadata.h b/src/trace_processor/storage/metadata.h
index fa2643d..1663ff3 100644
--- a/src/trace_processor/storage/metadata.h
+++ b/src/trace_processor/storage/metadata.h
@@ -38,7 +38,6 @@
F(benchmark_story_run_index, KeyType::kSingle, Variadic::kInt), \
F(benchmark_story_run_time_us, KeyType::kSingle, Variadic::kInt), \
F(benchmark_story_tags, KeyType::kMulti, Variadic::kString), \
- F(android_packages_list, KeyType::kMulti, Variadic::kInt), \
F(statsd_triggering_subscription_id, KeyType::kSingle, Variadic::kInt), \
F(trace_uuid, KeyType::kSingle, Variadic::kString), \
F(system_name, KeyType::kSingle, Variadic::kString), \
diff --git a/src/trace_processor/storage/trace_storage.h b/src/trace_processor/storage/trace_storage.h
index 53e9e12..5539579 100644
--- a/src/trace_processor/storage/trace_storage.h
+++ b/src/trace_processor/storage/trace_storage.h
@@ -508,6 +508,13 @@
return &heap_profile_allocation_table_;
}
+ const tables::PackageListTable& package_list_table() const {
+ return package_list_table_;
+ }
+ tables::PackageListTable* mutable_package_list_table() {
+ return &package_list_table_;
+ }
+
const tables::CpuProfileStackSampleTable& cpu_profile_stack_sample_table()
const {
return cpu_profile_stack_sample_table_;
@@ -756,6 +763,7 @@
&string_pool_, nullptr};
tables::CpuProfileStackSampleTable cpu_profile_stack_sample_table_{
&string_pool_, nullptr};
+ tables::PackageListTable package_list_table_{&string_pool_, nullptr};
// Symbol tables (mappings from frames to symbol names)
tables::SymbolTable symbol_table_{&string_pool_, nullptr};
diff --git a/src/trace_processor/tables/profiler_tables.h b/src/trace_processor/tables/profiler_tables.h
index e96772b..b58c908 100644
--- a/src/trace_processor/tables/profiler_tables.h
+++ b/src/trace_processor/tables/profiler_tables.h
@@ -23,6 +23,17 @@
namespace trace_processor {
namespace tables {
+#define PERFETTO_TP_PACKAGES_LIST_DEF(NAME, PARENT, C) \
+ NAME(PackageListTable, "package_list") \
+ PERFETTO_TP_ROOT_TABLE(PARENT, C) \
+ C(StringPool::Id, package_name) \
+ C(int64_t, uid) \
+ C(int32_t, debuggable) \
+ C(int32_t, profileable_from_shell) \
+ C(int64_t, version_code)
+
+PERFETTO_TP_TABLE(PERFETTO_TP_PACKAGES_LIST_DEF);
+
#define PERFETTO_TP_STACK_PROFILE_MAPPING_DEF(NAME, PARENT, C) \
NAME(StackProfileMappingTable, "stack_profile_mapping") \
PERFETTO_TP_ROOT_TABLE(PARENT, C) \
diff --git a/src/trace_processor/tables/table_destructors.cc b/src/trace_processor/tables/table_destructors.cc
index 11e78b0..8e6dd44 100644
--- a/src/trace_processor/tables/table_destructors.cc
+++ b/src/trace_processor/tables/table_destructors.cc
@@ -53,6 +53,7 @@
HeapGraphObjectTable::~HeapGraphObjectTable() = default;
HeapGraphReferenceTable::~HeapGraphReferenceTable() = default;
VulkanMemoryAllocationsTable::~VulkanMemoryAllocationsTable() = default;
+PackageListTable::~PackageListTable() = default;
// slice_tables.h
SliceTable::~SliceTable() = default;
diff --git a/src/trace_processor/trace_processor_context.h b/src/trace_processor/trace_processor_context.h
index 64c2cbc..1f5cd64 100644
--- a/src/trace_processor/trace_processor_context.h
+++ b/src/trace_processor/trace_processor_context.h
@@ -29,6 +29,7 @@
namespace trace_processor {
class ArgsTracker;
+class AndroidProbesTracker;
class ChunkedTraceReader;
class ClockTracker;
class EventTracker;
@@ -79,11 +80,12 @@
// type is only available in storage_full target. To access these fields use
// the GetOrCreate() method on their subclass type, e.g.
// SyscallTracker::GetOrCreate(context)
- std::unique_ptr<Destructible> syscall_tracker; // SyscallTracker
- std::unique_ptr<Destructible> sched_tracker; // SchedEventTracker
- std::unique_ptr<Destructible> systrace_parser; // SystraceParser
- std::unique_ptr<Destructible> heap_graph_tracker; // HeapGraphTracker
- std::unique_ptr<Destructible> json_tracker; // JsonTracker
+ std::unique_ptr<Destructible> android_probes_tracker; // AndroidProbesTracker
+ std::unique_ptr<Destructible> syscall_tracker; // SyscallTracker
+ std::unique_ptr<Destructible> sched_tracker; // SchedEventTracker
+ std::unique_ptr<Destructible> systrace_parser; // SystraceParser
+ std::unique_ptr<Destructible> heap_graph_tracker; // HeapGraphTracker
+ std::unique_ptr<Destructible> json_tracker; // JsonTracker
// These fields are trace readers which will be called by |forwarding_parser|
// once the format of the trace is discovered. They are placed here as they
diff --git a/src/trace_processor/trace_processor_impl.cc b/src/trace_processor/trace_processor_impl.cc
index 9c24644..e90f50d 100644
--- a/src/trace_processor/trace_processor_impl.cc
+++ b/src/trace_processor/trace_processor_impl.cc
@@ -535,6 +535,7 @@
RegisterDbTable(storage->stack_profile_callsite_table());
RegisterDbTable(storage->stack_profile_mapping_table());
RegisterDbTable(storage->stack_profile_frame_table());
+ RegisterDbTable(storage->package_list_table());
RegisterDbTable(storage->android_log_table());
