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

Commit bd641e82 authored by Rupesh Bansal's avatar Rupesh Bansal
Browse files

Moved BatteryStats and AppOps wakelock updates to the notifier thread 

We are moving the tasks performed on AppOpsManager and BatteryStats to
the handler thread to improve the latency of acquiring and releasing
the wakelocks.

This CL was originally merged and then reverted: ag/27435252 because of
a regression that was introduced in Notifier::notifyWakeLockListener
where some required logic was put behind a feature flag.

Bug: 296508214
Test: atest NotifierTest
Change-Id: Id69ecb9c2cd90ee5342783ea1b220c8290212e81
parent b6778b2e

services/core/java/com/android/server/power/Notifier.java

+114 −48
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
package com.android.server.power;



import android.annotation.Nullable;

import android.annotation.SuppressLint;

import android.annotation.UserIdInt;

import android.app.ActivityManagerInternal;

import android.app.AppOpsManager;
@@ -197,9 +198,10 @@ public class Notifier { 
            FaceDownDetector faceDownDetector, ScreenUndimDetector screenUndimDetector,

            Executor backgroundExecutor, PowerManagerFlags powerManagerFlags, Injector injector) {

        mContext = context;

        mInjector = (injector == null) ? new RealInjector() : injector;

        mFlags = powerManagerFlags;

        mBatteryStats = batteryStats;

        mAppOps = mContext.getSystemService(AppOpsManager.class);

        mAppOps = mInjector.getAppOpsManager(context);

        mSuspendBlocker = suspendBlocker;

        mPolicy = policy;

        mFaceDownDetector = faceDownDetector;
@@ -230,7 +232,6 @@ public class Notifier { 
        mShowWirelessChargingAnimationConfig = context.getResources().getBoolean(

                com.android.internal.R.bool.config_showBuiltinWirelessChargingAnim);



        mInjector = (injector == null) ? new RealInjector() : injector;

        mWakeLockLog = mInjector.getWakeLockLog(context);

        // Initialize interactive state for battery stats.

        try {
@@ -264,6 +265,7 @@ public class Notifier { 
    /**

     * Called when a wake lock is acquired.

     */

    @SuppressLint("AndroidFrameworkRequiresPermission")

    public void onWakeLockAcquired(int flags, String tag, String packageName,

            int ownerUid, int ownerPid, WorkSource workSource, String historyTag,

            IWakeLockCallback callback) {
@@ -273,7 +275,9 @@ public class Notifier { 
                    + ", ownerUid=" + ownerUid + ", ownerPid=" + ownerPid

                    + ", workSource=" + workSource);

        }

        notifyWakeLockListener(callback, tag, true, ownerUid, flags);

        notifyWakeLockListener(callback, tag, true, ownerUid, ownerPid, flags, workSource,

                packageName, historyTag);

        if (!mFlags.improveWakelockLatency()) {

            final int monitorType = getBatteryStatsWakeLockMonitorType(flags);

            if (monitorType >= 0) {

                try {
@@ -286,14 +290,13 @@ public class Notifier { 
                        mBatteryStats.noteStartWakelock(ownerUid, ownerPid, tag, historyTag,

                                monitorType, unimportantForLogging);

                        // XXX need to deal with disabled operations.

                    mAppOps.startOpNoThrow(AppOpsManager.OP_WAKE_LOCK, ownerUid, packageName);

                        mAppOps.startOpNoThrow(AppOpsManager.OP_WAKE_LOCK, ownerUid, packageName,

                                false, null, null);

                    }

                } catch (RemoteException ex) {

                    // Ignore

                }

            }



        if (!mFlags.improveWakelockLatency()) {

            mWakeLockLog.onWakeLockAcquired(tag, ownerUid, flags, /*eventTime=*/ -1);

        }

        mWakefulnessSessionObserver.onWakeLockAcquired(flags);
@@ -404,6 +407,7 @@ public class Notifier { 
    /**

     * Called when a wake lock is released.

     */

    @SuppressLint("AndroidFrameworkRequiresPermission")

    public void onWakeLockReleased(int flags, String tag, String packageName,

            int ownerUid, int ownerPid, WorkSource workSource, String historyTag,

            IWakeLockCallback callback, int releaseReason) {
@@ -413,7 +417,9 @@ public class Notifier { 
                    + ", ownerUid=" + ownerUid + ", ownerPid=" + ownerPid

                    + ", workSource=" + workSource);

        }

        notifyWakeLockListener(callback, tag, false, ownerUid, flags);

        notifyWakeLockListener(callback, tag, false, ownerUid, ownerPid, flags, workSource,

                packageName, historyTag);

        if (!mFlags.improveWakelockLatency()) {

            final int monitorType = getBatteryStatsWakeLockMonitorType(flags);

            if (monitorType >= 0) {

                try {
@@ -423,13 +429,12 @@ public class Notifier { 
                    } else {

                        mBatteryStats.noteStopWakelock(ownerUid, ownerPid, tag,

                                historyTag, monitorType);

                    mAppOps.finishOp(AppOpsManager.OP_WAKE_LOCK, ownerUid, packageName);

                        mAppOps.finishOp(AppOpsManager.OP_WAKE_LOCK, ownerUid, packageName, null);

                    }

                } catch (RemoteException ex) {

                    // Ignore

                }

            }

        if (!mFlags.improveWakelockLatency()) {

            mWakeLockLog.onWakeLockReleased(tag, ownerUid, /*eventTime=*/ -1);

        }

        mWakefulnessSessionObserver.onWakeLockReleased(flags, releaseReason);
@@ -1049,24 +1054,75 @@ public class Notifier { 
    }



    private void notifyWakeLockListener(IWakeLockCallback callback, String tag, boolean isEnabled,

            int ownerUid, int flags) {

        if (callback != null) {

            long currentTime = mInjector.currentTimeMillis();

            int ownerUid, int ownerPid, int flags, WorkSource workSource, String packageName,

            String historyTag) {

        mHandler.post(() -> {

                try {

            if (mFlags.improveWakelockLatency()) {

                long currentTime = mInjector.currentTimeMillis();

                if (isEnabled) {

                            mWakeLockLog.onWakeLockAcquired(tag, ownerUid, flags, currentTime);

                    notifyWakelockAcquisition(tag, ownerUid, ownerPid, flags,

                            workSource, packageName, historyTag, currentTime);

                } else {

                            mWakeLockLog.onWakeLockReleased(tag, ownerUid, currentTime);

                    notifyWakelockRelease(tag, ownerUid, ownerPid, flags,

                            workSource, packageName, historyTag, currentTime);

                }

            }



            if (callback != null) {

                try {

                    callback.onStateChanged(isEnabled);

                } catch (RemoteException e) {

                    Slog.e(TAG, "Wakelock.mCallback [" + tag + "] is already dead.", e);

                }

            }

        });

    }



    @SuppressLint("AndroidFrameworkRequiresPermission")

    private void notifyWakelockAcquisition(String tag, int ownerUid, int ownerPid, int flags,

            WorkSource workSource, String packageName, String historyTag, long currentTime) {

        final int monitorType = getBatteryStatsWakeLockMonitorType(flags);

        if (monitorType >= 0) {

            try {

                final boolean unimportantForLogging = ownerUid == Process.SYSTEM_UID

                        && (flags & PowerManager.UNIMPORTANT_FOR_LOGGING) != 0;

                if (workSource != null) {

                    mBatteryStats.noteStartWakelockFromSource(workSource, ownerPid, tag,

                            historyTag, monitorType, unimportantForLogging);

                } else {

                    mBatteryStats.noteStartWakelock(ownerUid, ownerPid, tag, historyTag,

                            monitorType, unimportantForLogging);

                    // XXX need to deal with disabled operations.

                    mAppOps.startOpNoThrow(

                            AppOpsManager.OP_WAKE_LOCK, ownerUid, packageName,

                            false, null, null);

                }

            } catch (RemoteException ex) {

                // Do Nothing

            }

        }

        mWakeLockLog.onWakeLockAcquired(tag, ownerUid, flags, currentTime);

    }



    @SuppressLint("AndroidFrameworkRequiresPermission")

    private void notifyWakelockRelease(String tag, int ownerUid, int ownerPid, int flags,

            WorkSource workSource, String packageName, String historyTag, long currentTime) {

        final int monitorType = getBatteryStatsWakeLockMonitorType(flags);

        if (monitorType >= 0) {

            try {

                if (workSource != null) {

                    mBatteryStats.noteStopWakelockFromSource(workSource, ownerPid, tag,

                            historyTag, monitorType);

                } else {

                    mBatteryStats.noteStopWakelock(ownerUid, ownerPid, tag,

                            historyTag, monitorType);

                    mAppOps.finishOp(AppOpsManager.OP_WAKE_LOCK, ownerUid, packageName, null);

                }

            } catch (RemoteException ex) {

                // Ignore

            }

        }

        mWakeLockLog.onWakeLockReleased(tag, ownerUid, currentTime);

    }



    private final class NotifierHandler extends Handler {
@@ -1114,6 +1170,11 @@ public class Notifier { 
         * Gets the WakeLockLog object

         */

        WakeLockLog getWakeLockLog(Context context);



        /**

         * Gets the AppOpsManager system service

         */

        AppOpsManager getAppOpsManager(Context context);

    }



    static class RealInjector implements Injector {
@@ -1126,5 +1187,10 @@ public class Notifier { 
        public WakeLockLog getWakeLockLog(Context context) {

            return new WakeLockLog(context);

        }



        @Override

        public AppOpsManager getAppOpsManager(Context context) {

            return context.getSystemService(AppOpsManager.class);

        }

    }

}

services/tests/powerservicetests/src/com/android/server/power/NotifierTest.java

+122 −4
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; 
import static org.mockito.Mockito.verifyZeroInteractions;

import static org.mockito.Mockito.when;



import android.app.AppOpsManager;

import android.content.Context;

import android.content.res.Resources;

import android.hardware.SensorManager;
@@ -41,7 +42,6 @@ import android.os.IWakeLockCallback; 
import android.os.Looper;

import android.os.PowerManager;

import android.os.RemoteException;

import android.os.ServiceManager;

import android.os.VibrationAttributes;

import android.os.Vibrator;

import android.os.test.TestLooper;
@@ -82,8 +82,12 @@ public class NotifierTest { 
    @Mock private StatusBarManagerInternal mStatusBarManagerInternal;

    @Mock private WakeLockLog mWakeLockLog;



    @Mock private IBatteryStats mBatteryStats;



    @Mock private PowerManagerFlags mPowerManagerFlags;



    @Mock private AppOpsManager mAppOpsManager;



    private PowerManagerService mService;

    private Context mContextSpy;

    private Resources mResourcesSpy;
@@ -230,7 +234,7 @@ public class NotifierTest { 
    public void testOnWakeLockListener_RemoteException_NoRethrow() {

        when(mPowerManagerFlags.improveWakelockLatency()).thenReturn(true);

        createNotifier();



        clearInvocations(mWakeLockLog, mBatteryStats, mAppOpsManager);

        IWakeLockCallback exceptingCallback = new IWakeLockCallback.Stub() {

            @Override public void onStateChanged(boolean enabled) throws RemoteException {

                throw new RemoteException("Just testing");
@@ -245,6 +249,7 @@ public class NotifierTest { 
        verifyZeroInteractions(mWakeLockLog);

        mTestLooper.dispatchAll();

        verify(mWakeLockLog).onWakeLockReleased("wakelockTag", uid, 1);



        mNotifier.onWakeLockAcquired(PowerManager.PARTIAL_WAKE_LOCK, "wakelockTag",

                "my.package.name", uid, pid, /* workSource= */ null, /* historyTag= */ null,

                exceptingCallback);
@@ -277,6 +282,115 @@ public class NotifierTest { 
        verify(mWakeLockLog).onWakeLockReleased("wakelockTag", uid, -1);

    }





    @Test

    public void testOnWakeLockListener_FullWakeLock_ProcessesOnHandler() throws RemoteException {

        when(mPowerManagerFlags.improveWakelockLatency()).thenReturn(true);

        createNotifier();



        IWakeLockCallback exceptingCallback = new IWakeLockCallback.Stub() {

            @Override public void onStateChanged(boolean enabled) throws RemoteException {

                throw new RemoteException("Just testing");

            }

        };

        clearInvocations(mWakeLockLog, mBatteryStats, mAppOpsManager);



        final int uid = 1234;

        final int pid = 5678;



        // Release the wakelock

        mNotifier.onWakeLockReleased(PowerManager.SCREEN_BRIGHT_WAKE_LOCK, "wakelockTag",

                "my.package.name", uid, pid, /* workSource= */ null, /* historyTag= */ null,

                exceptingCallback);



        // No interaction because we expect that to happen in async

        verifyZeroInteractions(mWakeLockLog, mBatteryStats, mAppOpsManager);



        // Progressing the looper, and validating all the interactions

        mTestLooper.dispatchAll();

        verify(mWakeLockLog).onWakeLockReleased("wakelockTag", uid, 1);

        verify(mBatteryStats).noteStopWakelock(uid, pid, "wakelockTag", /* historyTag= */ null,

                BatteryStats.WAKE_TYPE_FULL);

        verify(mAppOpsManager).finishOp(AppOpsManager.OP_WAKE_LOCK, uid,

                "my.package.name", null);



        clearInvocations(mWakeLockLog, mBatteryStats, mAppOpsManager);



        // Acquire the wakelock

        mNotifier.onWakeLockAcquired(PowerManager.SCREEN_BRIGHT_WAKE_LOCK, "wakelockTag",

                "my.package.name", uid, pid, /* workSource= */ null, /* historyTag= */ null,

                exceptingCallback);



        // No interaction because we expect that to happen in async

        verifyNoMoreInteractions(mWakeLockLog, mBatteryStats, mAppOpsManager);



        // Progressing the looper, and validating all the interactions

        mTestLooper.dispatchAll();

        verify(mWakeLockLog).onWakeLockAcquired("wakelockTag", uid,

                PowerManager.SCREEN_BRIGHT_WAKE_LOCK, 1);

        verify(mBatteryStats).noteStartWakelock(uid, pid, "wakelockTag", /* historyTag= */ null,

                BatteryStats.WAKE_TYPE_FULL, false);

        verify(mAppOpsManager).startOpNoThrow(AppOpsManager.OP_WAKE_LOCK, uid,

                "my.package.name", false, null, null);



        // Test with improveWakelockLatency flag false, hence the wakelock log will run on the same

        // thread

        clearInvocations(mWakeLockLog, mBatteryStats, mAppOpsManager);

        when(mPowerManagerFlags.improveWakelockLatency()).thenReturn(false);



        mNotifier.onWakeLockAcquired(PowerManager.SCREEN_BRIGHT_WAKE_LOCK, "wakelockTag",

                "my.package.name", uid, pid, /* workSource= */ null, /* historyTag= */ null,

                exceptingCallback);

        verify(mWakeLockLog).onWakeLockAcquired("wakelockTag", uid,

                PowerManager.SCREEN_BRIGHT_WAKE_LOCK, -1);



        mNotifier.onWakeLockReleased(PowerManager.SCREEN_BRIGHT_WAKE_LOCK, "wakelockTag",

                "my.package.name", uid, pid, /* workSource= */ null, /* historyTag= */ null,

                exceptingCallback);

        verify(mWakeLockLog).onWakeLockReleased("wakelockTag", uid, -1);

    }



    @Test

    public void testOnWakeLockListener_FullWakeLock_ProcessesInSync() throws RemoteException {

        createNotifier();



        IWakeLockCallback exceptingCallback = new IWakeLockCallback.Stub() {

            @Override public void onStateChanged(boolean enabled) throws RemoteException {

                throw new RemoteException("Just testing");

            }

        };

        clearInvocations(mWakeLockLog, mBatteryStats, mAppOpsManager);



        final int uid = 1234;

        final int pid = 5678;



        // Release the wakelock

        mNotifier.onWakeLockReleased(PowerManager.SCREEN_BRIGHT_WAKE_LOCK, "wakelockTag",

                "my.package.name", uid, pid, /* workSource= */ null, /* historyTag= */ null,

                exceptingCallback);



        verify(mWakeLockLog).onWakeLockReleased("wakelockTag", uid, -1);

        verify(mBatteryStats).noteStopWakelock(uid, pid, "wakelockTag", /* historyTag= */ null,

                BatteryStats.WAKE_TYPE_FULL);

        verify(mAppOpsManager).finishOp(AppOpsManager.OP_WAKE_LOCK, uid,

                "my.package.name", null);



        clearInvocations(mWakeLockLog, mBatteryStats, mAppOpsManager);



        // Acquire the wakelock

        mNotifier.onWakeLockAcquired(PowerManager.SCREEN_BRIGHT_WAKE_LOCK, "wakelockTag",

                "my.package.name", uid, pid, /* workSource= */ null, /* historyTag= */ null,

                exceptingCallback);



        mTestLooper.dispatchAll();

        verify(mWakeLockLog).onWakeLockAcquired("wakelockTag", uid,

                PowerManager.SCREEN_BRIGHT_WAKE_LOCK, -1);

        verify(mBatteryStats).noteStartWakelock(uid, pid, "wakelockTag", /* historyTag= */ null,

                BatteryStats.WAKE_TYPE_FULL, false);

        verify(mAppOpsManager).startOpNoThrow(AppOpsManager.OP_WAKE_LOCK, uid,

                "my.package.name", false, null, null);

    }



    private final PowerManagerService.Injector mInjector = new PowerManagerService.Injector() {

        @Override

        Notifier createNotifier(Looper looper, Context context, IBatteryStats batteryStats,
@@ -365,13 +479,17 @@ public class NotifierTest { 
            public WakeLockLog getWakeLockLog(Context context) {

                return mWakeLockLog;

            }



            @Override

            public AppOpsManager getAppOpsManager(Context context) {

                return mAppOpsManager;

            }

        };



        mNotifier = new Notifier(

                mTestLooper.getLooper(),

                mContextSpy,

                IBatteryStats.Stub.asInterface(ServiceManager.getService(

                        BatteryStats.SERVICE_NAME)),

                mBatteryStats,

                mInjector.createSuspendBlocker(mService, "testBlocker"),

                null,

                null,