commit 6af1847b8ea55d9f21e75b0f0ed91dc0e8f68c32
author Eric Tan <taneric@google.com> Mon Aug 20 09:27:50 2018 -0700
committer Eric Tan <taneric@google.com> Tue Aug 21 09:35:41 2018 -0700
tree e88798e13894f9bb17f3ff310b160ebf0baa1b5d
parent 5786e01d64860448d93b681a59c7d5a60bc12298

NBLog: Make separate readers for dumping logs and histograms

Prior to this commit, the dumping of log contents and histograms from
PerformanceAnalysis were in conflict because histograms obtained data
from the merger's buffer, and the merger used the same readers as the
log dumps. One read would consume the whole buffer, which would cause
either a lossy dump or an lossy merge. Now, both operations can exist
without loss of information from sharing the same reader.

The step of merging contents of each writer's local buffer into a bigger
buffer has effectively been removed. PerformanceAnalysis now directly
reads from each writer's buffer instead of reading from the merger's
bigger buffer.

Test: dumpsys media.log -r
Bug: 68148948
Change-Id: I6d8ea6a8f6a43555183a6d8f17af567506a102f1

services/medialog/MediaLogService.cpp

diff --git a/services/medialog/MediaLogService.cpp b/services/medialog/MediaLogService.cpp
index 095dd5c..b23832e 100644
--- a/services/medialog/MediaLogService.cpp
+++ b/services/medialog/MediaLogService.cpp
@@ -58,9 +58,10 @@
             shared->size() < NBLog::Timeline::sharedSize(size)) {
         return;
     }
-    sp<NBLog::Reader> reader(new NBLog::Reader(shared, size, name));
+    sp<NBLog::Reader> reader(new NBLog::Reader(shared, size, name)); // Reader handled by merger
+    sp<NBLog::DumpReader> dumpReader(new NBLog::DumpReader(shared, size, name)); // for dumpsys
     Mutex::Autolock _l(mLock);
-    mReaders.add(reader);
+    mDumpReaders.add(dumpReader);
     mMerger.addReader(reader);
 }
 
@@ -70,9 +71,10 @@
         return;
     }
     Mutex::Autolock _l(mLock);
-    for (size_t i = 0; i < mReaders.size(); ) {
-        if (mReaders[i]->isIMemory(shared)) {
-            mReaders.removeAt(i);
+    for (size_t i = 0; i < mDumpReaders.size(); ) {
+        if (mDumpReaders[i]->isIMemory(shared)) {
+            mDumpReaders.removeAt(i);
+            // TODO mMerger.removeReaders(shared)
         } else {
             i++;
         }
@@ -120,18 +122,18 @@
                 return NO_ERROR;
             }
 
-            for (auto reader : mReaders) {
+            for (const auto &dumpReader : mDumpReaders) {
                 if (fd >= 0) {
-                    dprintf(fd, "\n%s:\n", reader->name().c_str());
-                    reader->dump(fd, 0 /*indent*/);
+                    dprintf(fd, "\n%s:\n", dumpReader->name().c_str());
+                    dumpReader->dump(fd, 0 /*indent*/);
                 } else {
-                    ALOGI("%s:", reader->name().c_str());
+                    ALOGI("%s:", dumpReader->name().c_str());
                 }
             }
             mLock.unlock();
         }
     }
-    //mMergeReader.dump(fd);
+    mMergeReader.dump(fd);
     return NO_ERROR;
 }