Optimize incidentd memory usage
1. Remove dependency of libprotobuf-cpp-lite, saves .so mmap ~200KB
2. Don't use auto except iterator for readability.
Bug: 74254200
Test: adb shell dumpsys meminfo `pid incidentd`
Change-Id: If6198521c3b80929d6ea3f7ed466b5195991ccfd
diff --git a/cmds/incidentd/Android.mk b/cmds/incidentd/Android.mk
index 008a1bf..db864ae 100644
--- a/cmds/incidentd/Android.mk
+++ b/cmds/incidentd/Android.mk
@@ -41,20 +41,16 @@
LOCAL_CFLAGS += \
-Os
endif
-
LOCAL_C_INCLUDES += $(LOCAL_PATH)/src
LOCAL_SHARED_LIBRARIES := \
libbase \
libbinder \
- libcutils \
libdebuggerd_client \
libdumputils \
libincident \
liblog \
- libprotobuf-cpp-lite \
libprotoutil \
- libselinux \
libservices \
libutils
@@ -122,14 +118,12 @@
LOCAL_SHARED_LIBRARIES := \
libbase \
libbinder \
- libcutils \
libdebuggerd_client \
libdumputils \
libincident \
liblog \
libprotobuf-cpp-lite \
libprotoutil \
- libselinux \
libservices \
libutils \
diff --git a/cmds/incidentd/src/IncidentService.cpp b/cmds/incidentd/src/IncidentService.cpp
index 28fb38a..d02b4dd 100644
--- a/cmds/incidentd/src/IncidentService.cpp
+++ b/cmds/incidentd/src/IncidentService.cpp
@@ -358,8 +358,7 @@
return error;
}
fprintf(err, "Read %zu bytes\n", buf.size());
- auto data = buf.data();
- PrivacyBuffer pBuf(p, data);
+ PrivacyBuffer pBuf(p, buf.data());
PrivacySpec spec = PrivacySpec::new_spec(argCount > 3 ? atoi(args[3]) : -1);
error = pBuf.strip(spec);
diff --git a/cmds/incidentd/src/PrivacyBuffer.cpp b/cmds/incidentd/src/PrivacyBuffer.cpp
index 73ee1cb..f1f7c589 100644
--- a/cmds/incidentd/src/PrivacyBuffer.cpp
+++ b/cmds/incidentd/src/PrivacyBuffer.cpp
@@ -101,7 +101,7 @@
}
// ================================================================================
-PrivacyBuffer::PrivacyBuffer(const Privacy* policy, EncodedBuffer::iterator& data)
+PrivacyBuffer::PrivacyBuffer(const Privacy* policy, EncodedBuffer::iterator data)
: mPolicy(policy), mData(data), mProto(), mSize(0) {}
PrivacyBuffer::~PrivacyBuffer() {}
diff --git a/cmds/incidentd/src/PrivacyBuffer.h b/cmds/incidentd/src/PrivacyBuffer.h
index 92e1a25..cd29d8b 100644
--- a/cmds/incidentd/src/PrivacyBuffer.h
+++ b/cmds/incidentd/src/PrivacyBuffer.h
@@ -34,7 +34,7 @@
*/
class PrivacyBuffer {
public:
- PrivacyBuffer(const Privacy* policy, EncodedBuffer::iterator& data);
+ PrivacyBuffer(const Privacy* policy, EncodedBuffer::iterator data);
~PrivacyBuffer();
/**
@@ -60,7 +60,7 @@
private:
const Privacy* mPolicy;
- EncodedBuffer::iterator& mData;
+ EncodedBuffer::iterator mData;
ProtoOutputStream mProto;
size_t mSize;
diff --git a/cmds/incidentd/src/Reporter.cpp b/cmds/incidentd/src/Reporter.cpp
index 12764f8..fc8a6db 100644
--- a/cmds/incidentd/src/Reporter.cpp
+++ b/cmds/incidentd/src/Reporter.cpp
@@ -89,11 +89,11 @@
IncidentMetadata::SectionStats* ReportRequestSet::sectionStats(int id) {
if (mSectionStats.find(id) == mSectionStats.end()) {
- auto stats = mMetadata.add_sections();
- stats->set_id(id);
+ IncidentMetadata::SectionStats stats;
+ stats.set_id(id);
mSectionStats[id] = stats;
}
- return mSectionStats[id];
+ return &mSectionStats[id];
}
// ================================================================================
@@ -182,7 +182,7 @@
}
// Execute - go get the data and write it into the file descriptors.
- auto stats = batch.sectionStats(id);
+ IncidentMetadata::SectionStats* stats = batch.sectionStats(id);
int64_t startTime = uptimeMillis();
err = (*section)->Execute(&batch);
int64_t endTime = uptimeMillis();
diff --git a/cmds/incidentd/src/Reporter.h b/cmds/incidentd/src/Reporter.h
index ba8965e..f9a092a 100644
--- a/cmds/incidentd/src/Reporter.h
+++ b/cmds/incidentd/src/Reporter.h
@@ -66,6 +66,7 @@
int mainFd() { return mMainFd; }
int mainDest() { return mMainDest; }
IncidentMetadata& metadata() { return mMetadata; }
+ map<int, IncidentMetadata::SectionStats>& allSectionStats() { return mSectionStats; }
bool containsSection(int id);
IncidentMetadata::SectionStats* sectionStats(int id);
@@ -77,7 +78,7 @@
int mMainDest;
IncidentMetadata mMetadata;
- map<int, IncidentMetadata::SectionStats*> mSectionStats;
+ map<int, IncidentMetadata::SectionStats> mSectionStats;
};
// ================================================================================
diff --git a/cmds/incidentd/src/Section.cpp b/cmds/incidentd/src/Section.cpp
index 6dd76a8..5cde5a9 100644
--- a/cmds/incidentd/src/Section.cpp
+++ b/cmds/incidentd/src/Section.cpp
@@ -109,7 +109,7 @@
EncodedBuffer::iterator data = buffer.data();
PrivacyBuffer privacyBuffer(get_privacy_of_section(id), data);
int writeable = 0;
- auto stats = requests->sectionStats(id);
+ IncidentMetadata::SectionStats* stats = requests->sectionStats(id);
stats->set_dump_size_bytes(data.size());
stats->set_dump_duration_ms(buffer.durationMs());
@@ -215,23 +215,48 @@
MetadataSection::~MetadataSection() {}
status_t MetadataSection::Execute(ReportRequestSet* requests) const {
- std::string metadataBuf;
- requests->metadata().SerializeToString(&metadataBuf);
+ ProtoOutputStream proto;
+ IncidentMetadata metadata = requests->metadata();
+ proto.write(FIELD_TYPE_ENUM | IncidentMetadata::kDestFieldNumber, metadata.dest());
+ proto.write(FIELD_TYPE_INT32 | IncidentMetadata::kRequestSizeFieldNumber,
+ metadata.request_size());
+ proto.write(FIELD_TYPE_BOOL | IncidentMetadata::kUseDropboxFieldNumber, metadata.use_dropbox());
+ for (auto iter = requests->allSectionStats().begin(); iter != requests->allSectionStats().end();
+ iter++) {
+ IncidentMetadata::SectionStats stats = iter->second;
+ uint64_t token = proto.start(FIELD_TYPE_MESSAGE | IncidentMetadata::kSectionsFieldNumber);
+ proto.write(FIELD_TYPE_INT32 | IncidentMetadata::SectionStats::kIdFieldNumber, stats.id());
+ proto.write(FIELD_TYPE_BOOL | IncidentMetadata::SectionStats::kSuccessFieldNumber,
+ stats.success());
+ proto.write(FIELD_TYPE_INT32 | IncidentMetadata::SectionStats::kReportSizeBytesFieldNumber,
+ stats.report_size_bytes());
+ proto.write(FIELD_TYPE_INT64 | IncidentMetadata::SectionStats::kExecDurationMsFieldNumber,
+ stats.exec_duration_ms());
+ proto.write(FIELD_TYPE_INT32 | IncidentMetadata::SectionStats::kDumpSizeBytesFieldNumber,
+ stats.dump_size_bytes());
+ proto.write(FIELD_TYPE_INT64 | IncidentMetadata::SectionStats::kDumpDurationMsFieldNumber,
+ stats.dump_duration_ms());
+ proto.write(FIELD_TYPE_BOOL | IncidentMetadata::SectionStats::kTimedOutFieldNumber,
+ stats.timed_out());
+ proto.write(FIELD_TYPE_BOOL | IncidentMetadata::SectionStats::kIsTruncatedFieldNumber,
+ stats.is_truncated());
+ proto.end(token);
+ }
+
for (ReportRequestSet::iterator it = requests->begin(); it != requests->end(); it++) {
const sp<ReportRequest> request = *it;
- if (metadataBuf.empty() || request->fd < 0 || request->err != NO_ERROR) {
+ if (request->fd < 0 || request->err != NO_ERROR) {
continue;
}
- write_section_header(request->fd, id, metadataBuf.size());
- if (!WriteFully(request->fd, (uint8_t const*)metadataBuf.data(), metadataBuf.size())) {
+ write_section_header(request->fd, id, proto.size());
+ if (!proto.flush(request->fd)) {
ALOGW("Failed to write metadata to fd %d", request->fd);
// we don't fail if we can't write to a single request's fd.
}
}
- if (requests->mainFd() >= 0 && !metadataBuf.empty()) {
- write_section_header(requests->mainFd(), id, metadataBuf.size());
- if (!WriteFully(requests->mainFd(), (uint8_t const*)metadataBuf.data(),
- metadataBuf.size())) {
+ if (requests->mainFd() >= 0) {
+ write_section_header(requests->mainFd(), id, proto.size());
+ if (!proto.flush(requests->mainFd())) {
ALOGW("Failed to write metadata to dropbox fd %d", requests->mainFd());
return -1;
}
diff --git a/cmds/incidentd/testdata/metadata.txt b/cmds/incidentd/testdata/metadata.txt
new file mode 100644
index 0000000..cb79a3d
--- /dev/null
+++ b/cmds/incidentd/testdata/metadata.txt
Binary files differ
diff --git a/cmds/incidentd/tests/Reporter_test.cpp b/cmds/incidentd/tests/Reporter_test.cpp
index 955dbac..42d94f0 100644
--- a/cmds/incidentd/tests/Reporter_test.cpp
+++ b/cmds/incidentd/tests/Reporter_test.cpp
@@ -191,7 +191,7 @@
reporter->batch.add(r);
ASSERT_EQ(Reporter::REPORT_FINISHED, reporter->runReport(&size));
- auto metadata = reporter->batch.metadata();
+ IncidentMetadata metadata = reporter->batch.metadata();
EXPECT_EQ(IncidentMetadata_Destination_EXPLICIT, metadata.dest());
EXPECT_EQ(1, metadata.request_size());
EXPECT_TRUE(metadata.use_dropbox());
diff --git a/cmds/incidentd/tests/Section_test.cpp b/cmds/incidentd/tests/Section_test.cpp
index 1528224..55192d0 100644
--- a/cmds/incidentd/tests/Section_test.cpp
+++ b/cmds/incidentd/tests/Section_test.cpp
@@ -48,6 +48,15 @@
public:
virtual void SetUp() override { ASSERT_NE(tf.fd, -1); }
+ void printDebugString(std::string s) {
+ fprintf(stderr, "size: %zu\n", s.length());
+ for (size_t i = 0; i < s.length(); i++) {
+ char c = s[i];
+ fprintf(stderr, "\\x%x", c);
+ }
+ fprintf(stderr, "\n");
+ }
+
protected:
TemporaryFile tf;
ReportRequestSet requests;
@@ -103,14 +112,18 @@
TEST_F(SectionTest, MetadataSection) {
MetadataSection ms;
+ const std::string testFile = kTestDataPath + "metadata.txt";
+ std::string expect;
+ ASSERT_TRUE(ReadFileToString(testFile, &expect));
requests.setMainFd(STDOUT_FILENO);
+ requests.setMainDest(android::os::DEST_LOCAL);
requests.sectionStats(1)->set_success(true);
CaptureStdout();
ASSERT_EQ(NO_ERROR, ms.Execute(&requests));
- EXPECT_THAT(GetCapturedStdout(), StrEq("\x12\b(\x1"
- "2\x4\b\x1\x10\x1"));
+ // Notice message_lite.h ParseFromString doesn't work so we just match the bytes directly.
+ EXPECT_THAT(GetCapturedStdout(), StrEq(expect));
}
TEST_F(SectionTest, FileSection) {