Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 56cc9276 authored by Milo Sredkov's avatar Milo Sredkov
Browse files

Reuse transaction-name cache across instances of the same interface. 

Currently every binder object stores its own mapping of transaction
codes to names.

This is OK for services but may be wasteful for things like callback
classes for which we can have many instances.

Also change the default getMaxTransactionId to be -1, as 0 is the
value for an interface with one method. There may be up to 50
interfaces in system_server for which we incorrectly assume that
they are user defined and allocate 1024 entries (wasting hundreds of
KB).

Bug: 330738126
Test: AidlTest
Flag: android.os.binder_cache_transaction_trace_names
Change-Id: I2b960c2a9f0f325e6d99bb0d15999997dbea81e2
parent 80ce1c97

core/java/android/os/Binder.java

+40 −7
Original line number Diff line number Diff line
@@ -46,6 +46,7 @@ import java.io.FileOutputStream; 
import java.io.IOException;

import java.io.PrintWriter;

import java.lang.reflect.Modifier;

import java.util.concurrent.ConcurrentHashMap;

import java.util.concurrent.atomic.AtomicReferenceArray;



/**
@@ -305,6 +306,15 @@ public class Binder implements IBinder { 
    private IInterface mOwner;

    @Nullable

    private String mDescriptor;



    /** A holder so we don't eagerly allocate the transaction trace names cache. */

    private static class TransactionTraceNamesCacheHolder {

        /** A map of the transaction names keyed by simple descriptors. */

        static final ConcurrentHashMap<String, AtomicReferenceArray<String>> sNamesCache =

                new ConcurrentHashMap<>();

    }



    /** Cached value to the above map. */

    private volatile AtomicReferenceArray<String> mTransactionTraceNames = null;

    private volatile String mSimpleDescriptor = null;

    private static final int TRANSACTION_TRACE_NAME_ID_LIMIT = 1024;
@@ -990,13 +1000,37 @@ public class Binder implements IBinder { 
     */

    @VisibleForTesting

    public final @Nullable String getTransactionTraceName(int transactionCode) {

        final boolean isInterfaceUserDefined = getMaxTransactionId() == 0;

        final boolean isInterfaceUserDefined = getMaxTransactionId() == -1;

        if (mTransactionTraceNames == null) {

            mSimpleDescriptor = getSimpleDescriptor();

            if (Flags.binderCacheTransactionTraceNames()) {

                // Prefer the full descriptor to avoid mixing up the method names for different

                // interfaces with the same simple descriptor.

                String key = mDescriptor != null ? mDescriptor : getClass().getName();

                // Check if we have it in the static cache already.

                var transactionTraceNames = TransactionTraceNamesCacheHolder.sNamesCache.get(key);

                if (transactionTraceNames == null) {

                    // Not in the static cache. Create a new array.

                    final int highestId = isInterfaceUserDefined ? TRANSACTION_TRACE_NAME_ID_LIMIT

                            : Math.min(getMaxTransactionId(), TRANSACTION_TRACE_NAME_ID_LIMIT);

                    transactionTraceNames = new AtomicReferenceArray(highestId + 1);

                    // Try to put it in the static cache.

                    var oldTransactionTraceNames =

                            TransactionTraceNamesCacheHolder.sNamesCache.putIfAbsent(

                                    key, transactionTraceNames);

                    if (oldTransactionTraceNames != null) {

                        // Another thread must have added an entry to the static cache in the mean

                        // time. Use the one already in the cache.

                        transactionTraceNames = oldTransactionTraceNames;

                    }

                }

                mTransactionTraceNames = transactionTraceNames;

            } else {

                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 = isInterfaceUserDefined

                ? transactionCode : transactionCode - FIRST_CALL_TRANSACTION;
@@ -1046,7 +1080,7 @@ public class Binder implements IBinder { 
     * @hide

     */

    public int getMaxTransactionId() {

        return 0;

        return -1;

    }



    /**
@@ -1407,7 +1441,6 @@ public class Binder implements IBinder { 
        // If the call was {@link IBinder#FLAG_ONEWAY} then these exceptions

        // disappear into the ether.

        final boolean tagEnabled = Trace.isTagEnabled(Trace.TRACE_TAG_AIDL);

        final boolean hasFullyQualifiedName = getMaxTransactionId() > 0;

        final String transactionTraceName;



        if (tagEnabled) {

core/java/android/os/flags.aconfig

+10 −0
Original line number Diff line number Diff line
@@ -406,6 +406,16 @@ flag { 
     bug: "418770969"

}



flag {

    namespace: "system_performance"

    name: "binder_cache_transaction_trace_names"

    description: "Reuse transaction names caches across binder instances of the same interface."

    bug: "330738126"

    metadata {

        purpose: PURPOSE_BUGFIX

    }

}



flag {

     namespace: "system_performance"

     name: "enable_has_binders"

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

+22 −1
Original line number Diff line number Diff line
@@ -20,9 +20,12 @@ import static org.junit.Assert.assertEquals; 
import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertNotSame;

import static org.junit.Assert.assertNull;

import static org.junit.Assert.assertSame;

import static org.junit.Assert.assertTrue;



import android.platform.test.annotations.DisabledOnRavenwood;

import android.platform.test.annotations.EnableFlags;

import android.platform.test.flag.junit.SetFlagsRule;

import android.platform.test.ravenwood.RavenwoodRule;



import androidx.test.filters.SmallTest;
@@ -40,13 +43,18 @@ public class AidlTest { 
    @Rule

    public final RavenwoodRule mRavenwood = new RavenwoodRule();



    @Rule

    public final SetFlagsRule mSetFlagsRule = new SetFlagsRule();



    private IAidlTest mRemote;

    private AidlObject mLocal;

    private AidlObject mLocal2;

    private NonAutoGeneratedObject mNonAutoGenerated;



    @Before

    public void setUp() throws Exception {

        mLocal = new AidlObject();

        mLocal2 = new AidlObject();

        mRemote = IAidlTest.Stub.asInterface(mLocal);

        mNonAutoGenerated = new NonAutoGeneratedObject("NonAutoGeneratedObject");

    }
@@ -469,10 +477,23 @@ public class AidlTest { 
                mLocal.getTransactionTraceName(IAidlTest.Stub.TRANSACTION_parcelableIn));

    }



    @Test

    @SmallTest

    @EnableFlags({Flags.FLAG_BINDER_CACHE_TRANSACTION_TRACE_NAMES})

    public void testGetTransactionNameReusedAcrossInstances() throws Exception {

        for (int transactionId = IBinder.FIRST_CALL_TRANSACTION;

                transactionId <= mLocal.getMaxTransactionId();

                transactionId++) {

            String name1 = mLocal.getTransactionTraceName(transactionId);

            String name2 = mLocal2.getTransactionTraceName(transactionId);

            assertSame(name1, name2);

        }

    }



    @Test

    @SmallTest

    public void testGetTransactionNameNonAutoGenerated() throws Exception {

        assertEquals(0, mNonAutoGenerated.getMaxTransactionId());

        assertEquals(-1, mNonAutoGenerated.getMaxTransactionId());



        assertEquals("AIDL::java::NonAutoGeneratedObject::#0::server",

                mNonAutoGenerated.getTransactionTraceName(0));