Better LocalLog

This patch fixes the following issues in LocalLog:
  - reverseDump() uses a descending iterator with linear complexity
    instead of a quadratic loop using get(index) on a linked list.
  - reverseDump() is added to ReadOnlyLocalLog.
  - synchronized section in log() is restricted to mutation of internal
    list.
  - formatting of the log message does not create an internal
    StringBuilder.
  - the instance variable mNow is removed: it was only used inside log()
    as a local variable.
  - remaining instance variables are qualified with final.
  - the linked list is replaced by a fixed capacity array-backed queue.

Test: added unit tests
Change-Id: I1a54f0ad26dd35448d3297ea24df1fd626d20ef3

core/java/android/util/LocalLog.java

@@ -20,44 +20,49 @@ import java.io.FileDescriptor;
 import java.io.PrintWriter;
 import java.util.Calendar;
 import java.util.Iterator;
-import java.util.LinkedList;
+import java.util.Deque;
+import java.util.ArrayDeque;
 
 /**
  * @hide
  */
 public final class LocalLog {
 
-    private LinkedList<String> mLog;
-    private int mMaxLines;
-    private long mNow;
+    private final Deque<String> mLog;
+    private final int mMaxLines;
 
     public LocalLog(int maxLines) {
-        mLog = new LinkedList<String>();
-        mMaxLines = maxLines;
+        mMaxLines = Math.max(0, maxLines);
+        mLog = new ArrayDeque<>(mMaxLines);
     }
 
-    public synchronized void log(String msg) {
-        if (mMaxLines > 0) {
-            mNow = System.currentTimeMillis();
-            StringBuilder sb = new StringBuilder();
-            Calendar c = Calendar.getInstance();
-            c.setTimeInMillis(mNow);
-            sb.append(String.format("%tm-%td %tH:%tM:%tS.%tL", c, c, c, c, c, c));
-            mLog.add(sb.toString() + " - " + msg);
-            while (mLog.size() > mMaxLines) mLog.remove();
+    public void log(String msg) {
+        if (mMaxLines <= 0) {
+            return;
         }
+        Calendar c = Calendar.getInstance();
+        c.setTimeInMillis(System.currentTimeMillis());
+        append(String.format("%tm-%td %tH:%tM:%tS.%tL - %s", c, c, c, c, c, c, msg));
+    }
+
+    private synchronized void append(String logLine) {
+        while (mLog.size() >= mMaxLines) {
+            mLog.remove();
+        }
+        mLog.add(logLine);
     }
 
     public synchronized void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
-        Iterator<String> itr = mLog.listIterator(0);
+        Iterator<String> itr = mLog.iterator();
         while (itr.hasNext()) {
             pw.println(itr.next());
         }
     }
 
     public synchronized void reverseDump(FileDescriptor fd, PrintWriter pw, String[] args) {
-        for (int i = mLog.size() - 1; i >= 0; i--) {
-            pw.println(mLog.get(i));
+        Iterator<String> itr = mLog.descendingIterator();
+        while (itr.hasNext()) {
+            pw.println(itr.next());
         }
     }
 
@@ -69,6 +74,9 @@ public final class LocalLog {
         public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
             mLog.dump(fd, pw, args);
         }
+        public void reverseDump(FileDescriptor fd, PrintWriter pw, String[] args) {
+            mLog.reverseDump(fd, pw, args);
+        }
     }
 
     public ReadOnlyLocalLog readOnlyLocalLog() {

core/tests/coretests/src/android/util/LocalLogTest.java

@@ -0,0 +1,110 @@
+/*
+ * Copyright (C) 2016 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.
+ */
+
+package android.util;
+
+import junit.framework.TestCase;
+
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+
+public class LocalLogTest extends TestCase {
+
+    public void testA() {
+        String[] lines = {
+            "foo",
+            "bar",
+            "baz"
+        };
+        String[] want = lines;
+        testcase(new LocalLog(10), lines, want);
+    }
+
+    public void testB() {
+        String[] lines = {
+            "foo",
+            "bar",
+            "baz"
+        };
+        String[] want = {};
+        testcase(new LocalLog(0), lines, want);
+    }
+
+    public void testC() {
+        String[] lines = {
+            "dropped",
+            "dropped",
+            "dropped",
+            "dropped",
+            "dropped",
+            "dropped",
+            "foo",
+            "bar",
+            "baz",
+        };
+        String[] want = {
+            "foo",
+            "bar",
+            "baz",
+        };
+        testcase(new LocalLog(3), lines, want);
+    }
+
+    void testcase(LocalLog logger, String[] input, String[] want) {
+        for (String l : input) {
+            logger.log(l);
+        }
+        verifyAllLines(want, dump(logger).split("\n"));
+        verifyAllLines(reverse(want), reverseDump(logger).split("\n"));
+    }
+
+    void verifyAllLines(String[] wantLines, String[] gotLines) {
+        for (int i = 0; i < wantLines.length; i++) {
+            String want = wantLines[i];
+            String got = gotLines[i];
+            String msg = String.format("%s did not contain %s", quote(got), quote(want));
+            assertTrue(msg, got.contains(want));
+        }
+    }
+
+    static String dump(LocalLog logger) {
+        StringWriter buffer = new StringWriter();
+        PrintWriter writer = new PrintWriter(buffer);
+        logger.dump(null, writer, new String[0]);
+        return buffer.toString();
+    }
+
+    static String reverseDump(LocalLog logger) {
+        StringWriter buffer = new StringWriter();
+        PrintWriter writer = new PrintWriter(buffer);
+        logger.reverseDump(null, writer, new String[0]);
+        return buffer.toString();
+    }
+
+    static String quote(String s) {
+        return '"' + s + '"';
+    }
+
+    static String[] reverse(String[] ary) {
+        List<String> ls = Arrays.asList(ary);
+        Collections.reverse(ls);
+        return  ls.toArray(new String[ary.length]);
+    }
+}