diff options
| author | 2018-03-07 11:36:57 -0800 | |
|---|---|---|
| committer | 2018-03-07 18:25:44 -0800 | |
| commit | 86dce413f808ca9ef160e8762f74deaafd7c23ae (patch) | |
| tree | ffb221260c9ec970166fad35665c094d9dc8df3d | |
| parent | 09ed26a046c3824d06c283459532f4c8ee517711 (diff) | |
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
| -rw-r--r-- | cmds/incidentd/Android.mk | 6 | ||||
| -rw-r--r-- | cmds/incidentd/src/IncidentService.cpp | 3 | ||||
| -rw-r--r-- | cmds/incidentd/src/PrivacyBuffer.cpp | 2 | ||||
| -rw-r--r-- | cmds/incidentd/src/PrivacyBuffer.h | 4 | ||||
| -rw-r--r-- | cmds/incidentd/src/Reporter.cpp | 8 | ||||
| -rw-r--r-- | cmds/incidentd/src/Reporter.h | 3 | ||||
| -rw-r--r-- | cmds/incidentd/src/Section.cpp | 45 | ||||
| -rw-r--r-- | cmds/incidentd/testdata/metadata.txt | bin | 0 -> 26 bytes | |||
| -rw-r--r-- | cmds/incidentd/tests/Reporter_test.cpp | 2 | ||||
| -rw-r--r-- | cmds/incidentd/tests/Section_test.cpp | 17 |
10 files changed, 61 insertions, 29 deletions
diff --git a/cmds/incidentd/Android.mk b/cmds/incidentd/Android.mk index 008a1bf8397e..db864aed6bdc 100644 --- a/cmds/incidentd/Android.mk +++ b/cmds/incidentd/Android.mk @@ -41,20 +41,16 @@ else 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_STATIC_LIBRARIES := \ 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 28fb38a79a35..d02b4dd99067 100644 --- a/cmds/incidentd/src/IncidentService.cpp +++ b/cmds/incidentd/src/IncidentService.cpp @@ -358,8 +358,7 @@ status_t IncidentService::cmd_privacy(FILE* in, FILE* out, FILE* err, Vector<Str 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 73ee1cbbd70d..f1f7c589cf43 100644 --- a/cmds/incidentd/src/PrivacyBuffer.cpp +++ b/cmds/incidentd/src/PrivacyBuffer.cpp @@ -101,7 +101,7 @@ status_t PrivacyBuffer::stripField(const Privacy* parentPolicy, const PrivacySpe } // ================================================================================ -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 92e1a2572465..cd29d8b68591 100644 --- a/cmds/incidentd/src/PrivacyBuffer.h +++ b/cmds/incidentd/src/PrivacyBuffer.h @@ -34,7 +34,7 @@ using namespace android::util; */ class PrivacyBuffer { public: - PrivacyBuffer(const Privacy* policy, EncodedBuffer::iterator& data); + PrivacyBuffer(const Privacy* policy, EncodedBuffer::iterator data); ~PrivacyBuffer(); /** @@ -60,7 +60,7 @@ public: 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 12764f8ff09c..fc8a6db59e4b 100644 --- a/cmds/incidentd/src/Reporter.cpp +++ b/cmds/incidentd/src/Reporter.cpp @@ -89,11 +89,11 @@ bool ReportRequestSet::containsSection(int id) { return mSections.containsSectio 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 @@ Reporter::run_report_status_t Reporter::runReport(size_t* reportByteSize) { } // 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 ba8965ef4a6c..f9a092a0ca78 100644 --- a/cmds/incidentd/src/Reporter.h +++ b/cmds/incidentd/src/Reporter.h @@ -66,6 +66,7 @@ public: 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 @@ private: 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 6dd76a8d0421..5cde5a94e2dd 100644 --- a/cmds/incidentd/src/Section.cpp +++ b/cmds/incidentd/src/Section.cpp @@ -109,7 +109,7 @@ static status_t write_report_requests(const int id, const FdBuffer& buffer, 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() : Section(FIELD_ID_INCIDENT_METADATA, 0) {} 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 Binary files differnew file mode 100644 index 000000000000..cb79a3d6677a --- /dev/null +++ b/cmds/incidentd/testdata/metadata.txt diff --git a/cmds/incidentd/tests/Reporter_test.cpp b/cmds/incidentd/tests/Reporter_test.cpp index 955dbac72ebe..42d94f09b508 100644 --- a/cmds/incidentd/tests/Reporter_test.cpp +++ b/cmds/incidentd/tests/Reporter_test.cpp @@ -191,7 +191,7 @@ TEST_F(ReporterTest, ReportMetadata) { 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 1528224447af..55192d02acd3 100644 --- a/cmds/incidentd/tests/Section_test.cpp +++ b/cmds/incidentd/tests/Section_test.cpp @@ -48,6 +48,15 @@ class SectionTest : public Test { 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, HeaderSection) { 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) { |