Merge "Enable perfetto tracing for non-autogenerated AIDL names" into main

core/java/android/os/Binder.java

@@ -943,16 +943,19 @@ public class Binder implements IBinder {
      * @hide
      */
     @VisibleForTesting
-    public final @NonNull String getTransactionTraceName(int transactionCode) {
+    public final @Nullable String getTransactionTraceName(int transactionCode) {
+        final boolean isInterfaceUserDefined = getMaxTransactionId() == 0;
         if (mTransactionTraceNames == null) {
-            final int highestId = Math.min(getMaxTransactionId(), TRANSACTION_TRACE_NAME_ID_LIMIT);
+            final int highestId = isInterfaceUserDefined ? TRANSACTION_TRACE_NAME_ID_LIMIT
+                    : Math.min(getMaxTransactionId(), TRANSACTION_TRACE_NAME_ID_LIMIT);
             mSimpleDescriptor = getSimpleDescriptor();
             mTransactionTraceNames = new AtomicReferenceArray(highestId + 1);
         }
 
-        final int index = transactionCode - FIRST_CALL_TRANSACTION;
+        final int index = isInterfaceUserDefined
-        if (index < 0 || index >= mTransactionTraceNames.length()) {
+                ? transactionCode : transactionCode - FIRST_CALL_TRANSACTION;
-            return mSimpleDescriptor + "#" + transactionCode;
+        if (index >= mTransactionTraceNames.length() || index < 0) {
+            return null;
         }
 
         String transactionTraceName = mTransactionTraceNames.getAcquire(index);
@@ -1317,19 +1320,9 @@ public class Binder implements IBinder {
         final boolean hasFullyQualifiedName = getMaxTransactionId() > 0;
         final String transactionTraceName;
 
-        if (tagEnabled && hasFullyQualifiedName) {
+        if (tagEnabled) {
             // If tracing enabled and we have a fully qualified name, fetch the name
             transactionTraceName = getTransactionTraceName(code);
-        } else if (tagEnabled && isStackTrackingEnabled()) {
-            // If tracing is enabled and we *don't* have a fully qualified name, fetch the
-            // 'best effort' name only for stack tracking. This works around noticeable perf impact
-            // on low latency binder calls (<100us). The tracing call itself is between (1-10us) and
-            // the perf impact can be quite noticeable while benchmarking such binder calls.
-            // The primary culprits are ContentProviders and Cursors which convenienty don't
-            // autogenerate their AIDL and hence will not have a fully qualified name.
-            //
-            // TODO(b/253426478): Relax this constraint after a more robust fix
-            transactionTraceName = getTransactionTraceName(code);
         } else {
             transactionTraceName = null;
         }

core/tests/coretests/src/android/os/AidlTest.java

@@ -28,12 +28,14 @@ public class AidlTest extends TestCase {
 
     private IAidlTest mRemote;
     private AidlObject mLocal;
+    private NonAutoGeneratedObject mNonAutoGenerated;
 
     @Override
     protected void setUp() throws Exception {
         super.setUp();
         mLocal = new AidlObject();
         mRemote = IAidlTest.Stub.asInterface(mLocal);
+        mNonAutoGenerated = new NonAutoGeneratedObject("NonAutoGeneratedObject");
     }
 
     private static boolean check(TestParcelable p, int n, String s) {
@@ -84,6 +86,12 @@ public class AidlTest extends TestCase {
         }
     }
 
+    private static class NonAutoGeneratedObject extends Binder {
+        NonAutoGeneratedObject(String descriptor) {
+            super(descriptor);
+        }
+    }
+
     private static class AidlObject extends IAidlTest.Stub {
         public IInterface queryLocalInterface(String descriptor) {
             // overriding this to return null makes asInterface always
@@ -194,7 +202,7 @@ public class AidlTest extends TestCase {
                 TestParcelable[] a1, TestParcelable[] a2) {
             return null;
         }
-        
+
         public void voidSecurityException() {
             throw new SecurityException("gotcha!");
         }
@@ -396,7 +404,7 @@ public class AidlTest extends TestCase {
         assertEquals("s2[1]", s2[1]);
         assertEquals("s2[2]", s2[2]);
     }
-    
+
     @SmallTest
     public void testVoidSecurityException() throws Exception {
         boolean good = false;
@@ -407,7 +415,7 @@ public class AidlTest extends TestCase {
         }
         assertEquals(good, true);
     }
-    
+
     @SmallTest
     public void testIntSecurityException() throws Exception {
         boolean good = false;
@@ -420,7 +428,7 @@ public class AidlTest extends TestCase {
     }
 
     @SmallTest
-    public void testGetTransactionName() throws Exception {
+    public void testGetTransactionNameAutoGenerated() throws Exception {
         assertEquals(15, mLocal.getMaxTransactionId());
 
         assertEquals("booleanArray",
@@ -430,12 +438,21 @@ public class AidlTest extends TestCase {
         assertEquals("parcelableIn",
                 mLocal.getTransactionName(IAidlTest.Stub.TRANSACTION_parcelableIn));
 
-        assertEquals("IAidlTest:booleanArray",
+        assertEquals("AIDL::java::IAidlTest::booleanArray::server",
                 mLocal.getTransactionTraceName(IAidlTest.Stub.TRANSACTION_booleanArray));
-        assertEquals("IAidlTest:voidSecurityException",
+        assertEquals("AIDL::java::IAidlTest::voidSecurityException::server",
                 mLocal.getTransactionTraceName(IAidlTest.Stub.TRANSACTION_voidSecurityException));
-        assertEquals("IAidlTest:parcelableIn",
+        assertEquals("AIDL::java::IAidlTest::parcelableIn::server",
                 mLocal.getTransactionTraceName(IAidlTest.Stub.TRANSACTION_parcelableIn));
     }
-}
 
+    @SmallTest
+    public void testGetTransactionNameNonAutoGenerated() throws Exception {
+        assertEquals(0, mNonAutoGenerated.getMaxTransactionId());
+
+        assertEquals("AIDL::java::NonAutoGeneratedObject::#0::server",
+                mNonAutoGenerated.getTransactionTraceName(0));
+        assertEquals("AIDL::java::NonAutoGeneratedObject::#1::server",
+                mNonAutoGenerated.getTransactionTraceName(1));
+    }
+}