Merge "Optimize incidentd memory usage" into pi-dev

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 \
 

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, buf.data());
-            PrivacyBuffer pBuf(p, data);
 
             PrivacySpec spec = PrivacySpec::new_spec(argCount > 3 ? atoi(args[3]) : -1);
             error = pBuf.strip(spec);

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() {}

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;

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();
+        IncidentMetadata::SectionStats stats;
-        stats->set_id(id);
+        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();

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;
 };
 
 // ================================================================================

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;
+    ProtoOutputStream proto;
-    requests->metadata().SerializeToString(&metadataBuf);
+    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());
+        write_section_header(request->fd, id, proto.size());
-        if (!WriteFully(request->fd, (uint8_t const*)metadataBuf.data(), metadataBuf.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()) {
+    if (requests->mainFd() >= 0) {
-        write_section_header(requests->mainFd(), id, metadataBuf.size());
+        write_section_header(requests->mainFd(), id, proto.size());
-        if (!WriteFully(requests->mainFd(), (uint8_t const*)metadataBuf.data(),
+        if (!proto.flush(requests->mainFd())) {
-                        metadataBuf.size())) {
             ALOGW("Failed to write metadata to dropbox fd %d", requests->mainFd());
             return -1;
         }

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());

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"
+    // Notice message_lite.h ParseFromString doesn't work so we just match the bytes directly.
-                                           "2\x4\b\x1\x10\x1"));
+    EXPECT_THAT(GetCapturedStdout(), StrEq(expect));
 }
 
 TEST_F(SectionTest, FileSection) {