From de310dba78fa7cd60ad01cefa7fadb6d48fffcaa Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Tue, 4 Oct 2016 11:36:11 +0900 Subject: [PATCH] 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 | 44 ++++--- .../src/android/util/LocalLogTest.java | 110 ++++++++++++++++++ 2 files changed, 136 insertions(+), 18 deletions(-) create mode 100644 core/tests/coretests/src/android/util/LocalLogTest.java diff --git a/core/java/android/util/LocalLog.java b/core/java/android/util/LocalLog.java index 39f66a5f18fcd..665c583a56aba 100644 --- a/core/java/android/util/LocalLog.java +++ b/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 mLog; - private int mMaxLines; - private long mNow; + private final Deque mLog; + private final int mMaxLines; public LocalLog(int maxLines) { - mLog = new LinkedList(); - 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 itr = mLog.listIterator(0); + Iterator 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 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() { diff --git a/core/tests/coretests/src/android/util/LocalLogTest.java b/core/tests/coretests/src/android/util/LocalLogTest.java new file mode 100644 index 0000000000000..a63c8a0849640 --- /dev/null +++ b/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 ls = Arrays.asList(ary); + Collections.reverse(ls); + return ls.toArray(new String[ary.length]); + } +}