Merge "Reduce overhead of writing logcat from protolog method"

7 files changed, 31 insertions, 32 deletions

diff --git a/core/java/com/android/internal/protolog/BaseProtoLogImpl.java b/core/java/com/android/internal/protolog/BaseProtoLogImpl.java
index 83309fc61009..cd55d3292217 100644
--- a/core/java/com/android/internal/protolog/BaseProtoLogImpl.java
+++ b/core/java/com/android/internal/protolog/BaseProtoLogImpl.java
@@ -32,6 +32,7 @@ import static com.android.internal.protolog.ProtoLogMessage.STR_PARAMS;
 import android.annotation.Nullable;
 import android.os.ShellCommand;
 import android.os.SystemClock;
+import android.text.TextUtils;
 import android.util.Slog;
 import android.util.proto.ProtoOutputStream;
 
@@ -44,7 +45,6 @@ import java.io.File;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.util.ArrayList;
-import java.util.IllegalFormatConversionException;
 import java.util.TreeMap;
 import java.util.stream.Collectors;
 
@@ -108,10 +108,14 @@ public class BaseProtoLogImpl {
             messageString = mViewerConfig.getViewerString(messageHash);
         }
         if (messageString != null) {
-            try {
-                message = String.format(messageString, args);
-            } catch (IllegalFormatConversionException ex) {
-                Slog.w(TAG, "Invalid ProtoLog format string.", ex);
+            if (args != null) {
+                try {
+                    message = TextUtils.formatSimple(messageString, args);
+                } catch (Exception ex) {
+                    Slog.w(TAG, "Invalid ProtoLog format string.", ex);
+                }
+            } else {
+                message = messageString;
             }
         }
         if (message == null) {
diff --git a/core/java/com/android/internal/protolog/common/LogDataType.java b/core/java/com/android/internal/protolog/common/LogDataType.java
index 651932a7ba7e..c05824a58a77 100644
--- a/core/java/com/android/internal/protolog/common/LogDataType.java
+++ b/core/java/com/android/internal/protolog/common/LogDataType.java
@@ -74,13 +74,10 @@ public class LogDataType {
                         types.add(LogDataType.BOOLEAN);
                         break;
                     case 'd':
-                    case 'o':
                     case 'x':
                         types.add(LogDataType.LONG);
                         break;
                     case 'f':
-                    case 'e':
-                    case 'g':
                         types.add(LogDataType.DOUBLE);
                         break;
                     case 's':
diff --git a/core/java/com/android/internal/protolog/common/ProtoLog.java b/core/java/com/android/internal/protolog/common/ProtoLog.java
index 01cc1ed37ad4..93765cdf0890 100644
--- a/core/java/com/android/internal/protolog/common/ProtoLog.java
+++ b/core/java/com/android/internal/protolog/common/ProtoLog.java
@@ -22,11 +22,12 @@ package com.android.internal.protolog.common;
  * a messageString, which is a format string for the log message (has to be a string literal or
  * a concatenation of string literals) and a vararg array of parameters for the formatter.
  *
- * The syntax for the message string is a subset of {@code java.util.Formatter} syntax.
+ * The syntax for the message string depends on
+ * {@link android.text.TextUtils#formatSimple(String, Object...)}}.
  * Supported conversions:
  * %b - boolean
- * %d, %o and %x - integral type (Short, Integer or Long)
- * %f, %e and %g - floating point type (Float or Double)
+ * %d %x - integral type (Short, Integer or Long)
+ * %f - floating point type (Float or Double)
  * %s - string
  * %% - a literal percent character
  * The width and precision modifiers are supported, argument_index and flags are not.
diff --git a/services/tests/wmtests/src/com/android/server/wm/ProtoLogIntegrationTest.java b/services/tests/wmtests/src/com/android/server/wm/ProtoLogIntegrationTest.java
index 578a43cba33d..af0d32c764a4 100644
--- a/services/tests/wmtests/src/com/android/server/wm/ProtoLogIntegrationTest.java
+++ b/services/tests/wmtests/src/com/android/server/wm/ProtoLogIntegrationTest.java
@@ -50,17 +50,17 @@ public class ProtoLogIntegrationTest {
         ProtoLogImpl mockedProtoLog = mock(ProtoLogImpl.class);
         runWith(mockedProtoLog, this::testProtoLog);
         verify(mockedProtoLog).log(eq(ProtoLogImpl.LogLevel.ERROR), eq(ProtoLogGroup.TEST_GROUP),
-                anyInt(), eq(0b0010101001010111),
+                anyInt(), eq(0b0010010111),
                 eq(com.android.internal.protolog.ProtoLogGroup.TEST_GROUP.isLogToLogcat()
-                        ? "Test completed successfully: %b %d %o %x %e %g %f %% %s"
+                        ? "Test completed successfully: %b %d %x %f %% %s"
                         : null),
-                eq(new Object[]{true, 1L, 2L, 3L, 0.4, 0.5, 0.6, "ok"}));
+                eq(new Object[]{true, 1L, 2L, 0.3, "ok"}));
     }
 
     private void testProtoLog() {
         ProtoLog.e(ProtoLogGroup.TEST_GROUP,
-                "Test completed successfully: %b %d %o %x %e %g %f %% %s.",
-                true, 1, 2, 3, 0.4, 0.5, 0.6, "ok");
+                "Test completed successfully: %b %d %x %f %% %s",
+                true, 1, 2, 0.3, "ok");
     }
 
     /**
diff --git a/tests/Internal/src/com/android/internal/protolog/ProtoLogImplTest.java b/tests/Internal/src/com/android/internal/protolog/ProtoLogImplTest.java
index 3db011683a86..2cdb94526839 100644
--- a/tests/Internal/src/com/android/internal/protolog/ProtoLogImplTest.java
+++ b/tests/Internal/src/com/android/internal/protolog/ProtoLogImplTest.java
@@ -201,24 +201,24 @@ public class ProtoLogImplTest {
 
     @Test
     public void log_logcatEnabledExternalMessage() {
-        when(mReader.getViewerString(anyInt())).thenReturn("test %b %d %% %o %x %e %g %s %f");
+        when(mReader.getViewerString(anyInt())).thenReturn("test %b %d %% 0x%x %s %f");
         ProtoLogImpl implSpy = Mockito.spy(mProtoLog);
         TestProtoLogGroup.TEST_GROUP.setLogToLogcat(true);
         TestProtoLogGroup.TEST_GROUP.setLogToProto(false);
 
         implSpy.log(
                 ProtoLogImpl.LogLevel.INFO, TestProtoLogGroup.TEST_GROUP, 1234, 4321, null,
-                new Object[]{true, 10000, 20000, 30000, 0.0001, 0.00002, "test", 0.000003});
+                new Object[]{true, 10000, 30000, "test", 0.000003});
 
         verify(implSpy).passToLogcat(eq(TestProtoLogGroup.TEST_GROUP.getTag()), eq(
                 ProtoLogImpl.LogLevel.INFO),
-                eq("test true 10000 % 47040 7530 1.000000e-04 2.00000e-05 test 0.000003"));
+                eq("test true 10000 % 0x7530 test 3.0E-6"));
         verify(mReader).getViewerString(eq(1234));
     }
 
     @Test
     public void log_logcatEnabledInvalidMessage() {
-        when(mReader.getViewerString(anyInt())).thenReturn("test %b %d %% %o %x %e %g %s %f");
+        when(mReader.getViewerString(anyInt())).thenReturn("test %b %d %% %x %s %f");
         ProtoLogImpl implSpy = Mockito.spy(mProtoLog);
         TestProtoLogGroup.TEST_GROUP.setLogToLogcat(true);
         TestProtoLogGroup.TEST_GROUP.setLogToProto(false);
diff --git a/tests/Internal/src/com/android/internal/protolog/common/LogDataTypeTest.java b/tests/Internal/src/com/android/internal/protolog/common/LogDataTypeTest.java
index e20ca3df57c7..9c2f74eabe02 100644
--- a/tests/Internal/src/com/android/internal/protolog/common/LogDataTypeTest.java
+++ b/tests/Internal/src/com/android/internal/protolog/common/LogDataTypeTest.java
@@ -36,15 +36,12 @@ import java.util.List;
 public class LogDataTypeTest {
     @Test
     public void parseFormatString() {
-        String str = "%b %d %o %x %f %e %g %s %%";
+        String str = "%b %d %x %f %s %%";
         List<Integer> out = LogDataType.parseFormatString(str);
         assertEquals(Arrays.asList(
                 LogDataType.BOOLEAN,
                 LogDataType.LONG,
                 LogDataType.LONG,
-                LogDataType.LONG,
-                LogDataType.DOUBLE,
-                LogDataType.DOUBLE,
                 LogDataType.DOUBLE,
                 LogDataType.STRING
         ), out);
diff --git a/tools/protologtool/tests/com/android/protolog/tool/LogParserTest.kt b/tools/protologtool/tests/com/android/protolog/tool/LogParserTest.kt
index 67a31da87081..512d90c725fe 100644
--- a/tools/protologtool/tests/com/android/protolog/tool/LogParserTest.kt
+++ b/tools/protologtool/tests/com/android/protolog/tool/LogParserTest.kt
@@ -84,8 +84,8 @@ class LogParserTest {
 
     @Test
     fun parse_formatting() {
-        config[123] = ViewerConfigParser.ConfigEntry("Test completed successfully: %b %d %% %o" +
-                " %x %e %g %s %f", "ERROR", "WindowManager")
+        config[123] = ViewerConfigParser.ConfigEntry("Test completed successfully: %b %d %%" +
+                " %x %s %f", "ERROR", "WindowManager")
 
         val logBuilder = ProtoLogFileProto.newBuilder()
         val logMessageBuilder = ProtoLogMessage.newBuilder()
@@ -93,21 +93,21 @@ class LogParserTest {
                 .setMessageHash(123)
                 .setElapsedRealtimeNanos(0)
                 .addBooleanParams(true)
-                .addAllSint64Params(listOf(1000, 20000, 300000))
-                .addAllDoubleParams(listOf(0.1, 0.00001, 1000.1))
+                .addAllSint64Params(listOf(1000, 20000))
+                .addDoubleParams(1000.1)
                 .addStrParams("test")
         logBuilder.addLog(logMessageBuilder.build())
 
         parser.parse(buildProtoInput(logBuilder), getConfigDummyStream(), printStream)
 
         assertEquals("${testDate(0)} ERROR WindowManager: Test completed successfully: " +
-                "true 1000 % 47040 493e0 1.000000e-01 1.00000e-05 test 1000.100000\n",
+                "true 1000 % 4e20 test 1000.100000\n",
                 outStream.toString())
     }
 
     @Test
     fun parse_invalidParamsTooMany() {
-        config[123] = ViewerConfigParser.ConfigEntry("Test completed successfully: %b %d %% %o",
+        config[123] = ViewerConfigParser.ConfigEntry("Test completed successfully: %b %d %%",
                 "ERROR", "WindowManager")
 
         val logBuilder = ProtoLogFileProto.newBuilder()
@@ -129,8 +129,8 @@ class LogParserTest {
 
     @Test
     fun parse_invalidParamsNotEnough() {
-        config[123] = ViewerConfigParser.ConfigEntry("Test completed successfully: %b %d %% %o" +
-                " %x %e %g %s %f", "ERROR", "WindowManager")
+        config[123] = ViewerConfigParser.ConfigEntry("Test completed successfully: %b %d %%" +
+                " %x %s %f", "ERROR", "WindowManager")
 
         val logBuilder = ProtoLogFileProto.newBuilder()
         val logMessageBuilder = ProtoLogMessage.newBuilder()