From c0d8b4eb0a047850c3aa58e0d20156ce478e470c Mon Sep 17 00:00:00 2001
From: Dmitri Plotnikov <dplotnikov@google.com>
Date: Wed, 22 May 2024 16:25:44 -0700
Subject: [PATCH] Make SystemHealthManager.takeUidSnapshot call async
Bug: 339041893
Flag: backstage_power com.android.server.power.optimization.oneway_battery_stats_service
Test: atest CtsOsTestCases:SystemHealthManagerTest
Change-Id: Id47990cf9c3a87c81525ef02df96c04c62f1422b
---
.../os/health/SystemHealthManager.java | 88 ++++++++++++++++---
.../android/internal/app/IBatteryStats.aidl | 7 ++
.../server/am/BatteryStatsService.java | 55 ++++++++++++
.../android/server/power/stats/flags.aconfig | 10 +++
4 files changed, 147 insertions(+), 13 deletions(-)
diff --git a/core/java/android/os/health/SystemHealthManager.java b/core/java/android/os/health/SystemHealthManager.java
index 322a8e62dbb3..deabfed365a6 100644
--- a/core/java/android/os/health/SystemHealthManager.java
+++ b/core/java/android/os/health/SystemHealthManager.java
@@ -33,13 +33,16 @@ import android.os.Process;
import android.os.RemoteException;
import android.os.ResultReceiver;
import android.os.ServiceManager;
+import android.os.SynchronousResultReceiver;
import com.android.internal.app.IBatteryStats;
+import com.android.server.power.optimization.Flags;
import java.util.Arrays;
import java.util.Comparator;
import java.util.List;
import java.util.concurrent.Executor;
+import java.util.concurrent.TimeoutException;
import java.util.function.Consumer;
/**
@@ -67,6 +70,14 @@ public class SystemHealthManager {
private final IPowerStatsService mPowerStats;
private List<PowerMonitor> mPowerMonitorsInfo;
private final Object mPowerMonitorsLock = new Object();
+ private static final long TAKE_UID_SNAPSHOT_TIMEOUT_MILLIS = 10_000;
+
+ private static class PendingUidSnapshots {
+ public int[] uids;
+ public SynchronousResultReceiver resultReceiver;
+ }
+
+ private final PendingUidSnapshots mPendingUidSnapshots = new PendingUidSnapshots();
/**
* Construct a new SystemHealthManager object.
@@ -111,12 +122,19 @@ public class SystemHealthManager {
* @see Process#myUid() Process.myUid()
*/
public HealthStats takeUidSnapshot(int uid) {
- try {
- final HealthStatsParceler parceler = mBatteryStats.takeUidSnapshot(uid);
- return parceler.getHealthStats();
- } catch (RemoteException ex) {
- throw new RuntimeException(ex);
+ if (!Flags.onewayBatteryStatsService()) {
+ try {
+ final HealthStatsParceler parceler = mBatteryStats.takeUidSnapshot(uid);
+ return parceler.getHealthStats();
+ } catch (RemoteException ex) {
+ throw ex.rethrowFromSystemServer();
+ }
}
+ final HealthStats[] result = takeUidSnapshots(new int[]{uid});
+ if (result != null && result.length >= 1) {
+ return result[0];
+ }
+ return null;
}
/**
@@ -144,17 +162,61 @@ public class SystemHealthManager {
* other than its own.
*/
public HealthStats[] takeUidSnapshots(int[] uids) {
+ if (!Flags.onewayBatteryStatsService()) {
+ try {
+ final HealthStatsParceler[] parcelers = mBatteryStats.takeUidSnapshots(uids);
+ final int count = uids.length;
+ final HealthStats[] results = new HealthStats[count];
+ for (int i = 0; i < count; i++) {
+ results[i] = parcelers[i].getHealthStats();
+ }
+ return results;
+ } catch (RemoteException ex) {
+ throw ex.rethrowFromSystemServer();
+ }
+ }
+
+ SynchronousResultReceiver resultReceiver;
+ synchronized (mPendingUidSnapshots) {
+ if (Arrays.equals(mPendingUidSnapshots.uids, uids)) {
+ resultReceiver = mPendingUidSnapshots.resultReceiver;
+ } else {
+ mPendingUidSnapshots.uids = Arrays.copyOf(uids, uids.length);
+ mPendingUidSnapshots.resultReceiver = resultReceiver =
+ new SynchronousResultReceiver("takeUidSnapshots");
+ try {
+ mBatteryStats.takeUidSnapshotsAsync(uids, resultReceiver);
+ } catch (RemoteException ex) {
+ throw ex.rethrowFromSystemServer();
+ }
+ }
+ }
+
+ SynchronousResultReceiver.Result result;
try {
- final HealthStatsParceler[] parcelers = mBatteryStats.takeUidSnapshots(uids);
- final HealthStats[] results = new HealthStats[uids.length];
- final int N = uids.length;
- for (int i = 0; i < N; i++) {
- results[i] = parcelers[i].getHealthStats();
+ result = resultReceiver.awaitResult(TAKE_UID_SNAPSHOT_TIMEOUT_MILLIS);
+ } catch (TimeoutException e) {
+ throw new RuntimeException(e);
+ } finally {
+ synchronized (mPendingUidSnapshots) {
+ if (mPendingUidSnapshots.resultReceiver == resultReceiver) {
+ mPendingUidSnapshots.uids = null;
+ mPendingUidSnapshots.resultReceiver = null;
+ }
+ }
+ }
+
+ final HealthStats[] results = new HealthStats[uids.length];
+ if (result.bundle != null) {
+ HealthStatsParceler[] parcelers = result.bundle.getParcelableArray(
+ IBatteryStats.KEY_UID_SNAPSHOTS, HealthStatsParceler.class);
+ if (parcelers != null && parcelers.length == uids.length) {
+ for (int i = 0; i < parcelers.length; i++) {
+ results[i] = parcelers[i].getHealthStats();
+ }
}
- return results;
- } catch (RemoteException ex) {
- throw new RuntimeException(ex);
}
+ return results;
}
/**
diff --git a/core/java/com/android/internal/app/IBatteryStats.aidl b/core/java/com/android/internal/app/IBatteryStats.aidl
index 99b3f9a16355..ebcae277c62b 100644
--- a/core/java/com/android/internal/app/IBatteryStats.aidl
+++ b/core/java/com/android/internal/app/IBatteryStats.aidl
@@ -21,6 +21,7 @@ import android.os.BatteryUsageStats;
import android.os.BatteryUsageStatsQuery;
import android.os.BluetoothBatteryStats;
import android.os.ParcelFileDescriptor;
+import android.os.ResultReceiver;
import android.os.WakeLockStats;
import android.os.WorkSource;
import android.os.connectivity.CellularBatteryStats;
@@ -33,6 +34,9 @@ import android.telephony.ModemActivityInfo;
import android.telephony.SignalStrength;
interface IBatteryStats {
+ /** @hide */
+ const String KEY_UID_SNAPSHOTS = "uid_snapshots";
+
// These first methods are also called by native code, so must
// be kept in sync with frameworks/native/libs/binder/include_batterystats/batterystats/IBatteryStats.h
@EnforcePermission("UPDATE_DEVICE_STATS")
@@ -256,6 +260,9 @@ interface IBatteryStats {
@PermissionManuallyEnforced
HealthStatsParceler[] takeUidSnapshots(in int[] uid);
+ @PermissionManuallyEnforced
+ oneway void takeUidSnapshotsAsync(in int[] uid, in ResultReceiver result);
+
@EnforcePermission("UPDATE_DEVICE_STATS")
oneway void noteBluetoothControllerActivity(in BluetoothActivityEnergyInfo info);
@EnforcePermission("UPDATE_DEVICE_STATS")
diff --git a/services/core/java/com/android/server/am/BatteryStatsService.java b/services/core/java/com/android/server/am/BatteryStatsService.java
index 7c0325e7376a..d642b02e23ea 100644
--- a/services/core/java/com/android/server/am/BatteryStatsService.java
+++ b/services/core/java/com/android/server/am/BatteryStatsService.java
@@ -60,6 +60,7 @@ import android.os.BatteryUsageStats;
import android.os.BatteryUsageStatsQuery;
import android.os.Binder;
import android.os.BluetoothBatteryStats;
+import android.os.Bundle;
import android.os.Handler;
import android.os.HandlerThread;
import android.os.IBinder;
@@ -71,6 +72,7 @@ import android.os.PowerManagerInternal;
import android.os.PowerSaveState;
import android.os.Process;
import android.os.RemoteException;
+import android.os.ResultReceiver;
import android.os.ServiceManager;
import android.os.SystemClock;
import android.os.Trace;
@@ -3337,6 +3339,59 @@ public final class BatteryStatsService extends IBatteryStats.Stub
}
}
+ /**
+ * Gets a snapshot of the system health for a number of uids.
+ */
+ @Override
+ public void takeUidSnapshotsAsync(int[] requestUids, ResultReceiver resultReceiver) {
+ if (!onlyCaller(requestUids)) {
+ mContext.enforceCallingOrSelfPermission(
+ android.Manifest.permission.BATTERY_STATS, null);
+ }
+
+ Future future;
+ if (shouldCollectExternalStats()) {
+ future = mWorker.scheduleSync("get-health-stats-for-uids",
+ BatteryExternalStatsWorker.UPDATE_ALL);
+ } else {
+ future = null;
+ }
+
+ mHandler.post(() -> {
+ if (future != null) {
+ try {
+ // Worker uses a separate thread pool, so waiting here won't cause a deadlock
+ future.get();
+ } catch (InterruptedException | ExecutionException e) {
+ Slog.e(TAG, "Sync failed", e);
+ }
+ }
+
+ final long ident = Binder.clearCallingIdentity();
+ int i = -1;
+ try {
+ final int count = requestUids.length;
+ final HealthStatsParceler[] results = new HealthStatsParceler[count];
+ synchronized (mStats) {
+ for (i = 0; i < count; i++) {
+ results[i] = getHealthStatsForUidLocked(requestUids[i]);
+ }
+ }
+ Bundle resultData = new Bundle(1);
+ resultData.putParcelableArray(IBatteryStats.KEY_UID_SNAPSHOTS, results);
+ resultReceiver.send(0, resultData);
+ } catch (Exception ex) {
+ if (DBG) {
+ Slog.d(TAG, "Crashed while returning results for takeUidSnapshots("
+ + Arrays.toString(requestUids) + ") i=" + i, ex);
+ }
+ throw ex;
+ } finally {
+ Binder.restoreCallingIdentity(ident);
+ }
+ });
+ }
+
private boolean shouldCollectExternalStats() {
return (SystemClock.elapsedRealtime() - mWorker.getLastCollectionTimeStamp())
> mStats.getExternalStatsCollectionRateLimitMs();
diff --git a/services/core/java/com/android/server/power/stats/flags.aconfig b/services/core/java/com/android/server/power/stats/flags.aconfig
index 9283fe9bb7b0..6a5a7ac8d240 100644
--- a/services/core/java/com/android/server/power/stats/flags.aconfig
+++ b/services/core/java/com/android/server/power/stats/flags.aconfig
@@ -37,3 +37,13 @@ flag {
description: "Feature flag for streamlined misc (excluding CPU, Cell, Wifi, BT) battery stats"
bug: "333941740"
}
+
+flag {
+ name: "oneway_battery_stats_service"
+ namespace: "backstage_power"
+ description: "Bugfix flag for locking issues and watchdog kills in BatteryStatsService"
+ bug: "330792526"
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
+}
--
GitLab