From dc3a78354e5a25432825ec0ef11cfcb6a75fad52 Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Tue, 29 Jun 2021 15:47:32 -0700 Subject: [PATCH] Address race condition in procstate time tracking The issue occurs when the application proc state changes rapidly, e.g. from FOREGROUND to BACKGROUND to CACHED. The original code would sometimes attribute the time slice to the wrong proc state. Bug: 192550308 Test: (on cuttlefish) atest --rerun-until-failure 300 FrameworksCoreTests:com.android.internal.os.BstatsCpuTimesValidationTest -- --abi x86_64 Change-Id: Ic22bbfa3aae701014fc016ec9e2d32b1c528d462 --- core/java/android/os/BatteryStats.java | 7 +++---- .../internal/os/BstatsCpuTimesValidationTest.java | 12 +++++++++--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/core/java/android/os/BatteryStats.java b/core/java/android/os/BatteryStats.java index 4c8297a57b696..fb9911811da92 100644 --- a/core/java/android/os/BatteryStats.java +++ b/core/java/android/os/BatteryStats.java @@ -16,7 +16,6 @@ package android.os; -import static android.app.ActivityManager.PROCESS_STATE_BOUND_TOP; import static android.os.BatteryStatsManager.NUM_WIFI_STATES; import static android.os.BatteryStatsManager.NUM_WIFI_SUPPL_STATES; @@ -903,9 +902,9 @@ public abstract class BatteryStats implements Parcelable { * is not attributed to any non-critical process states. */ public static final int[] CRITICAL_PROC_STATES = { - PROCESS_STATE_TOP, - PROCESS_STATE_BOUND_TOP, PROCESS_STATE_FOREGROUND_SERVICE, - PROCESS_STATE_FOREGROUND + Uid.PROCESS_STATE_TOP, + Uid.PROCESS_STATE_FOREGROUND_SERVICE, + Uid.PROCESS_STATE_FOREGROUND }; public abstract long getProcessStateTime(int state, long elapsedRealtimeUs, int which); diff --git a/core/tests/coretests/src/com/android/internal/os/BstatsCpuTimesValidationTest.java b/core/tests/coretests/src/com/android/internal/os/BstatsCpuTimesValidationTest.java index 1fc3a21183851..4ef45e2b126b7 100644 --- a/core/tests/coretests/src/com/android/internal/os/BstatsCpuTimesValidationTest.java +++ b/core/tests/coretests/src/com/android/internal/os/BstatsCpuTimesValidationTest.java @@ -584,7 +584,7 @@ public class BstatsCpuTimesValidationTest { actualCpuTimeMs += cpuTimesMs[i]; } assertApproximateValue("Incorrect total cpu time, " + msgCpuTimes, - 2 * WORK_DURATION_MS, actualCpuTimeMs); + WORK_DURATION_MS, actualCpuTimeMs); batteryOffScreenOn(); } finally { @@ -656,8 +656,14 @@ public class BstatsCpuTimesValidationTest { } } - private void assertApproximateValue(String errorPrefix, long expectedValue, long actualValue) { - assertValueRange(errorPrefix, actualValue, expectedValue * 0.5, expectedValue * 1.5); + private void assertApproximateValue(String errorPrefix, long expectedValueMs, + long actualValueMs) { + // Allow the actual value to be 1 second smaller than the expected. + // Also allow it to be up to 5 seconds larger, to accommodate the arbitrary + // latency introduced by BatteryExternalStatsWorker.scheduleReadProcStateCpuTimes + assertValueRange(errorPrefix, actualValueMs, + expectedValueMs - 1000, + expectedValueMs + 5000); } private void assertValueRange(String errorPrefix,