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

Commit e932c83e authored by Pablo Gamito's avatar Pablo Gamito
Browse files

Use a Handler for ProtoLogging instead of an ExecutorService

Replaced the single-thread ExecutorService with an Android Handler.

This change aligns with Android best practices for managing background
threads and ensures FIFO execution of tasks.

Test: atest TracingTests
Flag: EXEMPT refactor
Change-Id: I2937865120fc3c63e7b9b99800fa2b06bc9be1da
parent 6c28db66
Loading
Loading
Loading
Loading
+53 −60
Original line number Diff line number Diff line
@@ -46,6 +46,8 @@ import static android.internal.perfetto.protos.TracePacketOuterClass.TracePacket
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.internal.perfetto.protos.Protolog.ProtoLogViewerConfig.MessageData;
import android.os.Handler;
import android.os.HandlerThread;
import android.os.RemoteException;
import android.os.ServiceManager;
import android.os.ShellCommand;
@@ -74,7 +76,6 @@ import com.android.internal.protolog.common.LogLevel;

import java.io.PrintWriter;
import java.io.StringWriter;
import java.lang.StringBuilder;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
@@ -83,15 +84,9 @@ import java.util.Objects;
import java.util.Set;
import java.util.TreeMap;
import java.util.UUID;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

/**
@@ -122,22 +117,16 @@ public abstract class PerfettoProtoLogImpl extends IProtoLogClient.Stub implemen
    @NonNull
    private final ReadWriteLock mConfigUpdaterLock = new ReentrantReadWriteLock();

    private final Lock mBackgroundServiceLock = new ReentrantLock();

    // NOTE: This is a single-thread executor configured with a ThreadPoolExecutor and a
    // LinkedBlockingQueue. This ensures that tasks are executed in FIFO (First-In, First-Out)
    // order, which is crucial for operations like connecting to the configuration service before
    // other logging activities, and synchronizing queued logging tasks on tracing start and stop.
    // Configuration:
    //   corePoolSize: 1 (single thread)
    //   maximumPoolSize: 1 (single thread)
    //   keepAliveTime: 0L (threads do not time out)
    //   workQueue: LinkedBlockingQueue (unbounded, FIFO)
    @NonNull
    private final HandlerThread mBackgroundThread;

    // Handler associated with the mBackgroundThread, ensuring tasks are processed sequentially
    // on a single background thread. This is crucial for operations like connecting to the
    // configuration service before other logging activities, and synchronizing queued
    // logging tasks on tracing start and stop.
    @VisibleForTesting
    public final ExecutorService mBackgroundLoggingService =
            new ThreadPoolExecutor(1, 1,
                    0L, TimeUnit.MILLISECONDS,
                    new LinkedBlockingQueue<>());
    @NonNull
    public final Handler mBackgroundHandler;

    // Set to true once this is ready to accept protolog to logcat requests.
    private boolean mLogcatReady = false;
@@ -163,6 +152,10 @@ public abstract class PerfettoProtoLogImpl extends IProtoLogClient.Stub implemen
        mCacheUpdater = cacheUpdater;
        mConfigurationService = configurationService;

        mBackgroundThread = new HandlerThread("ProtoLogBackground");
        mBackgroundThread.start();
        mBackgroundHandler = Handler.createAsync(mBackgroundThread.getLooper());

        registerGroupsLocally(groups);
    }

@@ -186,7 +179,7 @@ public abstract class PerfettoProtoLogImpl extends IProtoLogClient.Stub implemen
        Objects.requireNonNull(mConfigurationService,
                "A null ProtoLog Configuration Service was provided!");

        mBackgroundLoggingService.execute(() -> {
        mBackgroundHandler.post(() -> {
            try {
                var args = createConfigurationServiceRegisterClientArgs();

@@ -217,7 +210,7 @@ public abstract class PerfettoProtoLogImpl extends IProtoLogClient.Stub implemen
        mDataSource.unregisterOnStopCallback(this);

        if (android.tracing.Flags.clientSideProtoLogging()) {
            mBackgroundLoggingService.execute(() -> {
            mBackgroundHandler.post(() -> {
                try {
                    mConfigurationService.unregisterClient(this);
                } catch (RemoteException e) {
@@ -225,7 +218,8 @@ public abstract class PerfettoProtoLogImpl extends IProtoLogClient.Stub implemen
                }
            });
        }
        mBackgroundLoggingService.shutdown();

        mBackgroundThread.quitSafely();
    }

    @NonNull
@@ -413,9 +407,7 @@ public abstract class PerfettoProtoLogImpl extends IProtoLogClient.Stub implemen
            } else {
                stacktrace = null;
            }
            mBackgroundServiceLock.lock();
            try {
                mBackgroundLoggingService.execute(() -> {
            mBackgroundHandler.post(() -> {
                try {
                    logToProto(logLevel, group, message, args, tsNanos,
                            stacktrace);
@@ -443,9 +435,6 @@ public abstract class PerfettoProtoLogImpl extends IProtoLogClient.Stub implemen
                    throw new RuntimeException(sb.toString(), e);
                }
            });
            } finally {
                mBackgroundServiceLock.unlock();
            }
        }
        if (group.isLogToLogcat()) {
            logToLogcat(group.getTag(), logLevel, message, args);
@@ -932,12 +921,16 @@ public abstract class PerfettoProtoLogImpl extends IProtoLogClient.Stub implemen
    }

    private void waitForExistingBackgroundTasksToComplete() {
        try {
            this.mBackgroundLoggingService.submit(() -> {
        final CountDownLatch latch = new CountDownLatch(1);
        mBackgroundHandler.post(() -> {
            Log.i(LOG_TAG, "Completed all pending background tasks");
            }).get();
        } catch (InterruptedException | ExecutionException e) {
            latch.countDown();
        });
        try {
            latch.await();
        } catch (InterruptedException e) {
            Log.e(LOG_TAG, "Failed to wait for tracing service background tasks to complete", e);
            Thread.currentThread().interrupt(); // Preserve interrupt status
        }
    }

+1 −1
Original line number Diff line number Diff line
@@ -207,7 +207,7 @@ public class ProcessedPerfettoProtoLogImpl extends PerfettoProtoLogImpl {
        // Load in background to avoid delay in boot process.
        // The caveat is that any log message that is also logged to logcat will not be
        // successfully decoded until this completes.
        mBackgroundLoggingService.execute(() -> {
        mBackgroundHandler.post(() -> {
            mViewerConfigReader.loadViewerConfig(groupsLoggingToLogcat.toArray(new String[0]));
            readyToLogToLogcat();
        });
+2 −2
Original line number Diff line number Diff line
@@ -975,7 +975,7 @@ public class ProcessedPerfettoProtoLogImplTest {
                    sProtoLog.isProtoEnabled());

            // Submit a task that will block the executor queue.
            sProtoLog.mBackgroundLoggingService.execute(() -> {
            sProtoLog.mBackgroundHandler.post(() -> {
                try {
                    blockingTaskStartedExecution.set(true);
                    processingHasStartedLatch.countDown(); // Signal that this task has started
@@ -1017,7 +1017,7 @@ public class ProcessedPerfettoProtoLogImplTest {
            allowProcessingToContinueLatch.countDown();

            // Stop tracing immediately. The implementation should wait for the
            // mBackgroundLoggingService to process all queued messages (including the
            // mBackgroundHandler to process all queued messages (including the
            // now-unblocked first task and all subsequent log messages).
        } finally {
            // Ensure the latch is always counted down if an exception occurred before stop,