Skip to content

Commit 63e4c30

Browse files
laissandradelaissandrade
committed
Fixes in power benchmarks
- Add support check before running PowerHalAidlBenchmarks for setBoost and setMode, to avoid testing unsupported values and getting the HAL in a bad state. - Introduce loop breaks when a binder call fails, since SkipWithError is not enough to end the loop early and might cause test timeout if we try to interact with a HAL in a bad state. - Move PauseTiming/ResumeTimings to wrap one-way call delays only, as this API is expensive and should be used sparsely (and it's needed there to avoid filling up the one-way binder queue). - Make sure we close all sessions in createHintSession tests. - Use ndk::enum_range in AIDL enums to cover all available boost and mode values. Fix: 351008375 Flag: EXEMPT test only Change-Id: I6b277d60767f5928e397765b7dcabd709c4f8953 Test: libpowermanager_benchmarks
1 parent c09f9fa commit 63e4c30

3 files changed

Lines changed: 104 additions & 38 deletions

File tree

‎services/powermanager/benchmarks/PowerHalAidlBenchmarks.cpp‎

Lines changed: 58 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ using namespace android;
4040
using namespace std::chrono_literals;
4141

4242
// Values from Boost.aidl and Mode.aidl.
43-
static constexpr int64_t FIRST_BOOST = static_cast<int64_t>(Boost::INTERACTION);
44-
static constexpr int64_t LAST_BOOST = static_cast<int64_t>(Boost::CAMERA_SHOT);
45-
static constexpr int64_t FIRST_MODE = static_cast<int64_t>(Mode::DOUBLE_TAP_TO_WAKE);
46-
static constexpr int64_t LAST_MODE = static_cast<int64_t>(Mode::CAMERA_STREAMING_HIGH);
43+
static constexpr int64_t FIRST_BOOST = static_cast<int64_t>(*ndk::enum_range<Boost>().begin());
44+
static constexpr int64_t LAST_BOOST = static_cast<int64_t>(*(ndk::enum_range<Boost>().end()-1));
45+
static constexpr int64_t FIRST_MODE = static_cast<int64_t>(*ndk::enum_range<Mode>().begin());
46+
static constexpr int64_t LAST_MODE = static_cast<int64_t>(*(ndk::enum_range<Mode>().end()-1));
4747

4848
class DurationWrapper : public WorkDuration {
4949
public:
@@ -81,14 +81,17 @@ static void runBenchmark(benchmark::State& state, microseconds delay, R (IPower:
8181
return;
8282
}
8383

84-
while (state.KeepRunning()) {
84+
for (auto _ : state) {
8585
ret = (*hal.*fn)(std::forward<Args1>(args1)...);
86-
state.PauseTiming();
87-
if (!ret.isOk()) state.SkipWithError(ret.getDescription().c_str());
86+
if (!ret.isOk()) {
87+
state.SkipWithError(ret.getDescription().c_str());
88+
break;
89+
}
8890
if (delay > 0us) {
91+
state.PauseTiming();
8992
testDelaySpin(std::chrono::duration_cast<std::chrono::duration<float>>(delay).count());
93+
state.ResumeTiming();
9094
}
91-
state.ResumeTiming();
9295
}
9396
}
9497

@@ -123,14 +126,15 @@ static void runSessionBenchmark(benchmark::State& state, R (IPowerHintSession::*
123126
return;
124127
}
125128

126-
while (state.KeepRunning()) {
129+
for (auto _ : state) {
127130
ret = (*session.*fn)(std::forward<Args1>(args1)...);
128-
state.PauseTiming();
129-
if (!ret.isOk()) state.SkipWithError(ret.getDescription().c_str());
130-
if (ONEWAY_API_DELAY > 0us) {
131-
testDelaySpin(std::chrono::duration_cast<std::chrono::duration<float>>(ONEWAY_API_DELAY)
132-
.count());
131+
if (!ret.isOk()) {
132+
state.SkipWithError(ret.getDescription().c_str());
133+
break;
133134
}
135+
state.PauseTiming();
136+
testDelaySpin(std::chrono::duration_cast<std::chrono::duration<float>>(ONEWAY_API_DELAY)
137+
.count());
134138
state.ResumeTiming();
135139
}
136140
session->close();
@@ -150,11 +154,41 @@ static void BM_PowerHalAidlBenchmarks_isModeSupported(benchmark::State& state) {
150154

151155
static void BM_PowerHalAidlBenchmarks_setBoost(benchmark::State& state) {
152156
Boost boost = static_cast<Boost>(state.range(0));
157+
bool isSupported;
158+
std::shared_ptr<IPower> hal = PowerHalLoader::loadAidl();
159+
160+
if (hal == nullptr) {
161+
ALOGV("Power HAL not available, skipping test...");
162+
state.SkipWithMessage("Power HAL unavailable");
163+
return;
164+
}
165+
166+
ndk::ScopedAStatus ret = hal->isBoostSupported(boost, &isSupported);
167+
if (!ret.isOk() || !isSupported) {
168+
state.SkipWithMessage("operation unsupported");
169+
return;
170+
}
171+
153172
runBenchmark(state, ONEWAY_API_DELAY, &IPower::setBoost, boost, 1);
154173
}
155174

156175
static void BM_PowerHalAidlBenchmarks_setMode(benchmark::State& state) {
157176
Mode mode = static_cast<Mode>(state.range(0));
177+
bool isSupported;
178+
std::shared_ptr<IPower> hal = PowerHalLoader::loadAidl();
179+
180+
if (hal == nullptr) {
181+
ALOGV("Power HAL not available, skipping test...");
182+
state.SkipWithMessage("Power HAL unavailable");
183+
return;
184+
}
185+
186+
ndk::ScopedAStatus ret = hal->isModeSupported(mode, &isSupported);
187+
if (!ret.isOk() || !isSupported) {
188+
state.SkipWithMessage("operation unsupported");
189+
return;
190+
}
191+
158192
runBenchmark(state, ONEWAY_API_DELAY, &IPower::setMode, mode, false);
159193
}
160194

@@ -178,12 +212,20 @@ static void BM_PowerHalAidlBenchmarks_createHintSession(benchmark::State& state)
178212
ALOGV("Power HAL does not support this operation, skipping test...");
179213
state.SkipWithMessage("operation unsupported");
180214
return;
215+
} else if (!ret.isOk()) {
216+
state.SkipWithError(ret.getDescription().c_str());
217+
return;
218+
} else {
219+
appSession->close();
181220
}
182221

183-
while (state.KeepRunning()) {
222+
for (auto _ : state) {
184223
ret = hal->createHintSession(tgid, uid, threadIds, durationNanos, &appSession);
224+
if (!ret.isOk()) {
225+
state.SkipWithError(ret.getDescription().c_str());
226+
break;
227+
}
185228
state.PauseTiming();
186-
if (!ret.isOk()) state.SkipWithError(ret.getDescription().c_str());
187229
appSession->close();
188230
state.ResumeTiming();
189231
}

‎services/powermanager/benchmarks/PowerHalControllerBenchmarks.cpp‎

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@
1919
#include <aidl/android/hardware/power/Boost.h>
2020
#include <aidl/android/hardware/power/Mode.h>
2121
#include <benchmark/benchmark.h>
22+
#include <chrono>
2223
#include <powermanager/PowerHalController.h>
2324
#include <testUtil.h>
24-
#include <chrono>
2525

2626
using aidl::android::hardware::power::Boost;
2727
using aidl::android::hardware::power::Mode;
@@ -32,22 +32,38 @@ using namespace android;
3232
using namespace std::chrono_literals;
3333

3434
// Values from Boost.aidl and Mode.aidl.
35-
static constexpr int64_t FIRST_BOOST = static_cast<int64_t>(Boost::INTERACTION);
36-
static constexpr int64_t LAST_BOOST = static_cast<int64_t>(Boost::CAMERA_SHOT);
37-
static constexpr int64_t FIRST_MODE = static_cast<int64_t>(Mode::DOUBLE_TAP_TO_WAKE);
38-
static constexpr int64_t LAST_MODE = static_cast<int64_t>(Mode::CAMERA_STREAMING_HIGH);
35+
static constexpr int64_t FIRST_BOOST = static_cast<int64_t>(*ndk::enum_range<Boost>().begin());
36+
static constexpr int64_t LAST_BOOST = static_cast<int64_t>(*(ndk::enum_range<Boost>().end()-1));
37+
static constexpr int64_t FIRST_MODE = static_cast<int64_t>(*ndk::enum_range<Mode>().begin());
38+
static constexpr int64_t LAST_MODE = static_cast<int64_t>(*(ndk::enum_range<Mode>().end()-1));
3939

4040
// Delay between oneway method calls to avoid overflowing the binder buffers.
4141
static constexpr std::chrono::microseconds ONEWAY_API_DELAY = 100us;
4242

4343
template <typename T, class... Args0, class... Args1>
4444
static void runBenchmark(benchmark::State& state, HalResult<T> (PowerHalController::*fn)(Args0...),
4545
Args1&&... args1) {
46-
while (state.KeepRunning()) {
47-
PowerHalController controller;
46+
PowerHalController initController;
47+
HalResult<T> result = (initController.*fn)(std::forward<Args1>(args1)...);
48+
if (result.isFailed()) {
49+
state.SkipWithError(result.errorMessage());
50+
return;
51+
} else if (result.isUnsupported()) {
52+
ALOGV("Power HAL does not support this operation, skipping test...");
53+
state.SkipWithMessage("operation unsupported");
54+
return;
55+
}
56+
57+
for (auto _ : state) {
58+
PowerHalController controller; // new controller to avoid caching
4859
HalResult<T> ret = (controller.*fn)(std::forward<Args1>(args1)...);
60+
if (ret.isFailed()) {
61+
state.SkipWithError(ret.errorMessage());
62+
break;
63+
}
4964
state.PauseTiming();
50-
if (ret.isFailed()) state.SkipWithError("Power HAL request failed");
65+
testDelaySpin(
66+
std::chrono::duration_cast<std::chrono::duration<float>>(ONEWAY_API_DELAY).count());
5167
state.ResumeTiming();
5268
}
5369
}
@@ -57,22 +73,27 @@ static void runCachedBenchmark(benchmark::State& state,
5773
HalResult<T> (PowerHalController::*fn)(Args0...), Args1&&... args1) {
5874
PowerHalController controller;
5975
// First call out of test, to cache HAL service and isSupported result.
60-
(controller.*fn)(std::forward<Args1>(args1)...);
76+
HalResult<T> result = (controller.*fn)(std::forward<Args1>(args1)...);
77+
if (result.isFailed()) {
78+
state.SkipWithError(result.errorMessage());
79+
return;
80+
} else if (result.isUnsupported()) {
81+
ALOGV("Power HAL does not support this operation, skipping test...");
82+
state.SkipWithMessage("operation unsupported");
83+
return;
84+
}
6185

62-
while (state.KeepRunning()) {
86+
for (auto _ : state) {
6387
HalResult<T> ret = (controller.*fn)(std::forward<Args1>(args1)...);
64-
state.PauseTiming();
6588
if (ret.isFailed()) {
66-
state.SkipWithError("Power HAL request failed");
89+
state.SkipWithError(ret.errorMessage());
90+
break;
6791
}
68-
testDelaySpin(
69-
std::chrono::duration_cast<std::chrono::duration<float>>(ONEWAY_API_DELAY).count());
70-
state.ResumeTiming();
7192
}
7293
}
7394

7495
static void BM_PowerHalControllerBenchmarks_init(benchmark::State& state) {
75-
while (state.KeepRunning()) {
96+
for (auto _ : state) {
7697
PowerHalController controller;
7798
controller.init();
7899
}
@@ -90,12 +111,12 @@ static void BM_PowerHalControllerBenchmarks_initCached(benchmark::State& state)
90111

91112
static void BM_PowerHalControllerBenchmarks_setBoost(benchmark::State& state) {
92113
Boost boost = static_cast<Boost>(state.range(0));
93-
runBenchmark(state, &PowerHalController::setBoost, boost, 0);
114+
runBenchmark(state, &PowerHalController::setBoost, boost, 1);
94115
}
95116

96117
static void BM_PowerHalControllerBenchmarks_setBoostCached(benchmark::State& state) {
97118
Boost boost = static_cast<Boost>(state.range(0));
98-
runCachedBenchmark(state, &PowerHalController::setBoost, boost, 0);
119+
runCachedBenchmark(state, &PowerHalController::setBoost, boost, 1);
99120
}
100121

101122
static void BM_PowerHalControllerBenchmarks_setMode(benchmark::State& state) {

‎services/powermanager/benchmarks/PowerHalHidlBenchmarks.cpp‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,17 @@ static void runBenchmark(benchmark::State& state, microseconds delay, Return<R>
5454
return;
5555
}
5656

57-
while (state.KeepRunning()) {
57+
for (auto _ : state) {
5858
Return<R> ret = (*hal.*fn)(std::forward<Args1>(args1)...);
59-
state.PauseTiming();
60-
if (!ret.isOk()) state.SkipWithError(ret.description().c_str());
59+
if (!ret.isOk()) {
60+
state.SkipWithError(ret.description().c_str());
61+
break;
62+
}
6163
if (delay > 0us) {
64+
state.PauseTiming();
6265
testDelaySpin(std::chrono::duration_cast<std::chrono::duration<float>>(delay).count());
66+
state.ResumeTiming();
6367
}
64-
state.ResumeTiming();
6568
}
6669
}
6770

0 commit comments

Comments
 (0)