From a101c0af5a61e7ac925be09c899450e254db68ce Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Mon, 16 Nov 2020 14:37:47 -0800 Subject: [PATCH] Add a perf test for BatteryStatsHelper On Bramble, we get these results: endToEnd_median: 321.983605 getStats_median: 258.134786 getStats_cached_median: 48.506614 powerCalculation_median: 51.798291 Test: Lock the device to little cores for consistency $ ./frameworks/base/libs/hwui/tests/scripts/prep_generic.sh little $ atest BatteryStatsPerfTests Bug: 173446001 Change-Id: I307bcea38c7001d950de1b506c31b64f89629952 --- .../android/internal/app/IBatteryStats.aidl | 2 +- .../internal/os/BatteryStatsHelper.java | 19 ++- .../server/am/BatteryStatsService.java | 18 ++- tests/BatteryStatsPerfTest/Android.bp | 25 ++++ .../BatteryStatsPerfTest/AndroidManifest.xml | 27 ++++ tests/BatteryStatsPerfTest/AndroidTest.xml | 28 ++++ .../os/BatteryStatsHelperPerfTest.java | 135 ++++++++++++++++++ 7 files changed, 243 insertions(+), 11 deletions(-) create mode 100644 tests/BatteryStatsPerfTest/Android.bp create mode 100644 tests/BatteryStatsPerfTest/AndroidManifest.xml create mode 100644 tests/BatteryStatsPerfTest/AndroidTest.xml create mode 100644 tests/BatteryStatsPerfTest/src/com/android/internal/os/BatteryStatsHelperPerfTest.java diff --git a/core/java/com/android/internal/app/IBatteryStats.aidl b/core/java/com/android/internal/app/IBatteryStats.aidl index e6a166140d892..6d98a5982c4f9 100644 --- a/core/java/com/android/internal/app/IBatteryStats.aidl +++ b/core/java/com/android/internal/app/IBatteryStats.aidl @@ -52,7 +52,7 @@ interface IBatteryStats { @UnsupportedAppUsage byte[] getStatistics(); - ParcelFileDescriptor getStatisticsStream(); + ParcelFileDescriptor getStatisticsStream(boolean updateAll); // Return true if we see the battery as currently charging. @UnsupportedAppUsage(maxTargetSdk = 30, trackingBug = 170729553) diff --git a/core/java/com/android/internal/os/BatteryStatsHelper.java b/core/java/com/android/internal/os/BatteryStatsHelper.java index 267674592b02b..9e59e50cffd09 100644 --- a/core/java/com/android/internal/os/BatteryStatsHelper.java +++ b/core/java/com/android/internal/os/BatteryStatsHelper.java @@ -203,7 +203,7 @@ public class BatteryStatsHelper { } } return getStats(IBatteryStats.Stub.asInterface( - ServiceManager.getService(BatteryStats.SERVICE_NAME))); + ServiceManager.getService(BatteryStats.SERVICE_NAME)), true); } @UnsupportedAppUsage @@ -223,8 +223,13 @@ public class BatteryStatsHelper { @UnsupportedAppUsage public BatteryStats getStats() { + return getStats(true /* updateAll */); + } + + /** Retrieves stats from BatteryService, optionally getting updated numbers */ + public BatteryStats getStats(boolean updateAll) { if (mStats == null) { - load(); + load(updateAll); } return mStats; } @@ -720,19 +725,23 @@ public class BatteryStatsHelper { @UnsupportedAppUsage private void load() { + load(true); + } + + private void load(boolean updateAll) { if (mBatteryInfo == null) { return; } - mStats = getStats(mBatteryInfo); + mStats = getStats(mBatteryInfo, updateAll); if (mCollectBatteryBroadcast) { mBatteryBroadcast = mContext.registerReceiver(null, new IntentFilter(Intent.ACTION_BATTERY_CHANGED)); } } - private static BatteryStatsImpl getStats(IBatteryStats service) { + private static BatteryStatsImpl getStats(IBatteryStats service, boolean updateAll) { try { - ParcelFileDescriptor pfd = service.getStatisticsStream(); + ParcelFileDescriptor pfd = service.getStatisticsStream(updateAll); if (pfd != null) { if (false) { Log.d(TAG, "selinux context: " diff --git a/services/core/java/com/android/server/am/BatteryStatsService.java b/services/core/java/com/android/server/am/BatteryStatsService.java index c3ba15024a78a..ed2faf992e0e7 100644 --- a/services/core/java/com/android/server/am/BatteryStatsService.java +++ b/services/core/java/com/android/server/am/BatteryStatsService.java @@ -519,16 +519,24 @@ public final class BatteryStatsService extends IBatteryStats.Stub return data; } - public ParcelFileDescriptor getStatisticsStream() { + /** + * Returns parceled BatteryStats as a MemoryFile. + * + * @param forceUpdate If true, runs a sync to get fresh battery stats. Otherwise, + * returns the current values. + */ + public ParcelFileDescriptor getStatisticsStream(boolean forceUpdate) { mContext.enforceCallingOrSelfPermission( android.Manifest.permission.BATTERY_STATS, null); //Slog.i("foo", "SENDING BATTERY INFO:"); //mStats.dumpLocked(new LogPrinter(Log.INFO, "foo", Log.LOG_ID_SYSTEM)); Parcel out = Parcel.obtain(); - // Drain the handler queue to make sure we've handled all pending works, so we'll get - // an accurate stats. - awaitCompletion(); - syncStats("get-stats", BatteryExternalStatsWorker.UPDATE_ALL); + if (forceUpdate) { + // Drain the handler queue to make sure we've handled all pending works, so we'll get + // an accurate stats. + awaitCompletion(); + syncStats("get-stats", BatteryExternalStatsWorker.UPDATE_ALL); + } synchronized (mStats) { mStats.writeToParcel(out, 0); } diff --git a/tests/BatteryStatsPerfTest/Android.bp b/tests/BatteryStatsPerfTest/Android.bp new file mode 100644 index 0000000000000..58ccec7054195 --- /dev/null +++ b/tests/BatteryStatsPerfTest/Android.bp @@ -0,0 +1,25 @@ +// Copyright (C) 2020 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +android_test { + name: "BatteryStatsPerfTests", + srcs: ["src/**/*.java"], + static_libs: [ + "androidx.test.rules", + "apct-perftests-utils", + "truth-prebuilt", + ], + platform_apis: true, + certificate: "platform", +} diff --git a/tests/BatteryStatsPerfTest/AndroidManifest.xml b/tests/BatteryStatsPerfTest/AndroidManifest.xml new file mode 100644 index 0000000000000..7633d5283f5e2 --- /dev/null +++ b/tests/BatteryStatsPerfTest/AndroidManifest.xml @@ -0,0 +1,27 @@ + + + + + + + + + + + + diff --git a/tests/BatteryStatsPerfTest/AndroidTest.xml b/tests/BatteryStatsPerfTest/AndroidTest.xml new file mode 100644 index 0000000000000..2f9e114756d1b --- /dev/null +++ b/tests/BatteryStatsPerfTest/AndroidTest.xml @@ -0,0 +1,28 @@ + + + + + + + diff --git a/tests/BatteryStatsPerfTest/src/com/android/internal/os/BatteryStatsHelperPerfTest.java b/tests/BatteryStatsPerfTest/src/com/android/internal/os/BatteryStatsHelperPerfTest.java new file mode 100644 index 0000000000000..6266cda204b01 --- /dev/null +++ b/tests/BatteryStatsPerfTest/src/com/android/internal/os/BatteryStatsHelperPerfTest.java @@ -0,0 +1,135 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.internal.os; + +import static com.google.common.truth.Truth.assertThat; + +import android.content.Context; +import android.os.BatteryStats; +import android.os.Bundle; +import android.os.UserHandle; +import android.perftests.utils.BenchmarkState; +import android.perftests.utils.PerfStatusReporter; + +import androidx.test.InstrumentationRegistry; +import androidx.test.filters.LargeTest; +import androidx.test.runner.AndroidJUnit4; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.util.List; + +@RunWith(AndroidJUnit4.class) +@LargeTest +public class BatteryStatsHelperPerfTest { + + @Rule + public final PerfStatusReporter mPerfStatusReporter = new PerfStatusReporter(); + + /** + * Measures the performance of {@link BatteryStatsHelper#getStats()}, which triggers + * a battery stats sync on every iteration. + */ + @Test + public void testGetStats_forceUpdate() { + final Context context = InstrumentationRegistry.getContext(); + final BatteryStatsHelper statsHelper = new BatteryStatsHelper(context, + true /* collectBatteryBroadcast */); + statsHelper.create((Bundle) null); + statsHelper.refreshStats(BatteryStats.STATS_SINCE_CHARGED, UserHandle.myUserId()); + + final BenchmarkState state = mPerfStatusReporter.getBenchmarkState(); + while (state.keepRunning()) { + state.pauseTiming(); + statsHelper.clearStats(); + state.resumeTiming(); + + statsHelper.getStats(); + + assertThat(statsHelper.getUsageList()).isNotEmpty(); + } + } + + /** + * Measures performance of the {@link BatteryStatsHelper#getStats(boolean)}, which does + * not trigger a sync and just returns current values. + */ + @Test + public void testGetStats_cached() { + final Context context = InstrumentationRegistry.getContext(); + final BatteryStatsHelper statsHelper = new BatteryStatsHelper(context, + true /* collectBatteryBroadcast */); + statsHelper.create((Bundle) null); + statsHelper.refreshStats(BatteryStats.STATS_SINCE_CHARGED, UserHandle.myUserId()); + + final BenchmarkState state = mPerfStatusReporter.getBenchmarkState(); + while (state.keepRunning()) { + state.pauseTiming(); + statsHelper.clearStats(); + state.resumeTiming(); + + statsHelper.getStats(false /* forceUpdate */); + + assertThat(statsHelper.getUsageList()).isNotEmpty(); + } + } + + @Test + public void testPowerCalculation() { + final Context context = InstrumentationRegistry.getContext(); + final BatteryStatsHelper statsHelper = new BatteryStatsHelper(context, + true /* collectBatteryBroadcast */); + statsHelper.create((Bundle) null); + statsHelper.getStats(); + + final BenchmarkState state = mPerfStatusReporter.getBenchmarkState(); + while (state.keepRunning()) { + // This will use the cached BatteryStatsObject + statsHelper.refreshStats(BatteryStats.STATS_SINCE_CHARGED, UserHandle.myUserId()); + + assertThat(statsHelper.getUsageList()).isNotEmpty(); + } + } + + @Test + public void testEndToEnd() { + final Context context = InstrumentationRegistry.getContext(); + final BenchmarkState state = mPerfStatusReporter.getBenchmarkState(); + while (state.keepRunning()) { + final BatteryStatsHelper statsHelper = new BatteryStatsHelper(context, + true /* collectBatteryBroadcast */); + statsHelper.create((Bundle) null); + statsHelper.clearStats(); + statsHelper.refreshStats(BatteryStats.STATS_SINCE_CHARGED, UserHandle.myUserId()); + + state.pauseTiming(); + + List usageList = statsHelper.getUsageList(); + double power = 0; + for (int i = 0; i < usageList.size(); i++) { + BatterySipper sipper = usageList.get(i); + power += sipper.sumPower(); + } + + assertThat(power).isGreaterThan(0.0); + + state.resumeTiming(); + } + } +}