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

This commit is contained in:
Zimuzo Ezeozue
2023-07-25 22:26:24 +00:00
committed by Gerrit Code Review
2 changed files with 34 additions and 24 deletions

View File

@@ -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;
if (index < 0 || index >= mTransactionTraceNames.length()) {
return mSimpleDescriptor + "#" + transactionCode;
final int index = isInterfaceUserDefined
? transactionCode : transactionCode - FIRST_CALL_TRANSACTION;
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;
}

View File

@@ -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));
}
}