Remove unnecessary perfetto/perfprofd messages from statsd_config proto
This change simplifies the on-device statsd_config.proto when
it comes to handling perfetto and perfprofd configs.
In both cases statsd doesn't need to deserialize or know the schema
of the nested perfetto/perfprofd configs, because it just passes
the binary-encoded message to the corresponding daemons.
This change replaces the submessage with a generic "bytes" field.
As per https://developers.google.com/protocol-buffers/docs/proto#updating
"Embedded messages are compatible with bytes if the bytes contain an
encoded version of the message."
Advantages:
* One less copy of perfetto/perfprofd config around, reducing the risk
of getting that out of sync and the corresponding maintenance cost.
* Reduce the risk of failures within statsd if trying to parse an invalid
or outdated config proto.
* Reduce the CPU usage of statsd, by avoiding deserialization and
re-encoding of the config protos.
* Avoid bugs like b/111448265 where the binary size of statsd inflates
if the config protos are too big.
* Reflect what happens in the statsd code, specifically the fact that
statsd only sees the bytes of the config but doesn't touch/alter its
contents.
This change depends on: aosp/718808 and ag/4569627
Bug: 111448265
Test: Manual + CTS (atest AnomalyDetectionTests#testPerfetto)
Change-Id: I6ede5fa07c3ab3f71d29d38f2a40a90e88a16fd6
diff --git a/bin/Android.mk b/bin/Android.mk
index e182ad1..49ea6d5 100644
--- a/bin/Android.mk
+++ b/bin/Android.mk
@@ -60,7 +60,6 @@
src/metrics/MetricsManager.cpp \
src/metrics/metrics_manager_util.cpp \
src/packages/UidMap.cpp \
- src/perfetto/perfetto_config.proto \
src/storage/StorageManager.cpp \
src/StatsLogProcessor.cpp \
src/StatsService.cpp \
@@ -73,8 +72,7 @@
# TODO(b/110563449): Once statsd is using a blueprint file, migrate to the proper filegroups.
statsd_common_src += \
- ../../../../system/extras/perfprofd/binder_interface/aidl/android/os/IPerfProfd.aidl \
- src/perfprofd/perfprofd_config.proto
+ ../../../../system/extras/perfprofd/binder_interface/aidl/android/os/IPerfProfd.aidl
statsd_common_c_includes := \
$(LOCAL_PATH)/src \
@@ -258,8 +256,6 @@
src/metrics_constants/metrics_constants.proto \
src/stats_log.proto \
src/statsd_config.proto \
- src/perfetto/perfetto_config.proto \
- src/perfprofd/perfprofd_config.proto \
src/atoms.proto
LOCAL_PROTOC_OPTIMIZE_TYPE := lite
diff --git a/bin/src/external/Perfetto.cpp b/bin/src/external/Perfetto.cpp
index 0554483..e44351b 100644
--- a/bin/src/external/Perfetto.cpp
+++ b/bin/src/external/Perfetto.cpp
@@ -113,7 +113,7 @@
return false;
}
- std::string cfgProto = config.trace_config().SerializeAsString();
+ const std::string& cfgProto = config.trace_config();
size_t bytesWritten = fwrite(cfgProto.data(), 1, cfgProto.size(), writePipeStream);
fclose(writePipeStream);
if (bytesWritten != cfgProto.size() || cfgProto.size() == 0) {
diff --git a/bin/src/external/Perfprofd.cpp b/bin/src/external/Perfprofd.cpp
index ff237e8..1678f10 100644
--- a/bin/src/external/Perfprofd.cpp
+++ b/bin/src/external/Perfprofd.cpp
@@ -55,17 +55,8 @@
return false;
}
- // Add protobufs can't be described in AIDL, we need to re-serialize
- // the config proto to send it.
- std::vector<uint8_t> proto_serialized;
- {
- const auto& config_proto = config.perfprofd_config();
- int size = config_proto.ByteSize();
- proto_serialized.resize(size);
- ::google::protobuf::uint8* target_ptr =
- reinterpret_cast<::google::protobuf::uint8*>(proto_serialized.data());
- config_proto.SerializeWithCachedSizesToArray(target_ptr);
- }
+ auto* data = reinterpret_cast<const uint8_t*>(config.perfprofd_config().data());
+ std::vector<uint8_t> proto_serialized(data, data + config.perfprofd_config().size());
// TODO: alert-id etc?
diff --git a/bin/src/perfetto/perfetto_config.proto b/bin/src/perfetto/perfetto_config.proto
deleted file mode 100644
index 56d12f8..0000000
--- a/bin/src/perfetto/perfetto_config.proto
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * Copyright (C) 2017 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.
- */
-
-syntax = "proto2";
-
-package perfetto.protos;
-
-message DataSourceConfig {
- message FtraceConfig {
- repeated string event_names = 1;
- }
-
- optional string name = 1;
-
- optional uint32 target_buffer = 2;
-
- optional FtraceConfig ftrace_config = 100;
-}
-
-message TraceConfig {
- message BufferConfig {
- optional uint32 size_kb = 1;
-
- enum OptimizeFor {
- DEFAULT = 0;
- ONE_SHOT_READ = 1;
-
- }
- optional OptimizeFor optimize_for = 3;
-
- enum FillPolicy {
- UNSPECIFIED = 0;
- RING_BUFFER = 1;
- }
- optional FillPolicy fill_policy = 4;
- }
- repeated BufferConfig buffers = 1;
-
- message DataSource {
- optional protos.DataSourceConfig config = 1;
-
- repeated string producer_name_filter = 2;
- }
- repeated DataSource data_sources = 2;
-
- optional uint32 duration_ms = 3;
-}
diff --git a/bin/src/perfprofd/perfprofd_config.proto b/bin/src/perfprofd/perfprofd_config.proto
deleted file mode 120000
index c8be247..0000000
--- a/bin/src/perfprofd/perfprofd_config.proto
+++ /dev/null
@@ -1 +0,0 @@
-../../../../../../system/extras/perfprofd/perfprofd_config.proto
\ No newline at end of file
diff --git a/bin/src/statsd_config.proto b/bin/src/statsd_config.proto
index fabc5f9..26dfda3 100644
--- a/bin/src/statsd_config.proto
+++ b/bin/src/statsd_config.proto
@@ -21,9 +21,6 @@
option java_package = "com.android.internal.os";
option java_outer_classname = "StatsdConfigProto";
-import "frameworks/base/cmds/statsd/src/perfetto/perfetto_config.proto";
-import "frameworks/base/cmds/statsd/src/perfprofd/perfprofd_config.proto";
-
enum Position {
POSITION_UNKNOWN = 0;
@@ -304,11 +301,21 @@
}
message PerfettoDetails {
- optional perfetto.protos.TraceConfig trace_config = 1;
+ // The |trace_config| field is a proto-encoded message of type
+ // perfetto.protos.TraceConfig defined in
+ // //external/perfetto/protos/perfetto/config/. On device,
+ // statsd doesn't need to deserialize the message as it's just
+ // passed binary-encoded to the perfetto cmdline client.
+ optional bytes trace_config = 1;
}
message PerfprofdDetails {
- optional android.perfprofd.ProfilingConfig perfprofd_config = 1;
+ // The |perfprofd_config| field is a proto-encoded message of type
+ // android.perfprofd.ProfilingConfig defined in
+ // //system/extras/perfprofd/. On device, statsd doesn't need to
+ // deserialize the message as it's just passed binary-encoded to
+ // the perfprofd service.
+ optional bytes perfprofd_config = 1;
}
message BroadcastSubscriberDetails {
