From 56093a77a950660d0c281ecc7c5de57af88ae397 Mon Sep 17 00:00:00 2001 From: Matt Buckley Date: Mon, 7 Nov 2022 21:50:50 +0000 Subject: [PATCH] Add rate limiter to sendHint API call Adds a rate limiter to the HintSession.sendHint API for safety, and update tests to reflect this. Bug: b/243973548 Test: atest PerformanceHintNativeTestCases Test: atest FrameworksCoreTests:android.os.PerformanceHintManagerTest Change-Id: Ic68683bacf7df3e11efc3d59689b5470c3fa4274 --- .../os/PerformanceHintManagerTest.java | 3 +- native/android/Android.bp | 1 + native/android/performance_hint.cpp | 28 +++++++++++++++++-- .../PerformanceHintNativeTest.cpp | 11 ++++++-- 4 files changed, 37 insertions(+), 6 deletions(-) diff --git a/core/tests/coretests/src/android/os/PerformanceHintManagerTest.java b/core/tests/coretests/src/android/os/PerformanceHintManagerTest.java index d1d14f6fbcb49..44923b60fbb5d 100644 --- a/core/tests/coretests/src/android/os/PerformanceHintManagerTest.java +++ b/core/tests/coretests/src/android/os/PerformanceHintManagerTest.java @@ -117,7 +117,8 @@ public class PerformanceHintManagerTest { public void testSendHint() { Session s = createSession(); assumeNotNull(s); - s.sendHint(Session.CPU_LOAD_UP); + s.sendHint(Session.CPU_LOAD_RESET); + // ensure we can also send within the rate limit without exception s.sendHint(Session.CPU_LOAD_RESET); } diff --git a/native/android/Android.bp b/native/android/Android.bp index f1b1d79265ded..254eb4494ed8a 100644 --- a/native/android/Android.bp +++ b/native/android/Android.bp @@ -95,6 +95,7 @@ cc_library_shared { "libpowermanager", "android.hardware.configstore@1.0", "android.hardware.configstore-utils", + "android.hardware.power-V4-ndk", "libnativedisplay", ], diff --git a/native/android/performance_hint.cpp b/native/android/performance_hint.cpp index 40eb507a52139..9e97bd33ce9cd 100644 --- a/native/android/performance_hint.cpp +++ b/native/android/performance_hint.cpp @@ -16,6 +16,7 @@ #define LOG_TAG "perf_hint" +#include #include #include #include @@ -25,14 +26,21 @@ #include #include +#include #include #include using namespace android; using namespace android::os; +using namespace std::chrono_literals; + +using AidlSessionHint = aidl::android::hardware::power::SessionHint; + struct APerformanceHintSession; +constexpr int64_t SEND_HINT_TIMEOUT = std::chrono::nanoseconds(100ms).count(); + struct APerformanceHintManager { public: static APerformanceHintManager* getInstance(); @@ -75,6 +83,8 @@ private: int64_t mFirstTargetMetTimestamp; // Last target hit timestamp int64_t mLastTargetMetTimestamp; + // Last hint reported from sendHint indexed by hint value + std::vector mLastHintSentTimestamp; // Cached samples std::vector mActualDurationsNanos; std::vector mTimestampsNanos; @@ -147,7 +157,12 @@ APerformanceHintSession::APerformanceHintSession(sp session, mPreferredRateNanos(preferredRateNanos), mTargetDurationNanos(targetDurationNanos), mFirstTargetMetTimestamp(0), - mLastTargetMetTimestamp(0) {} + mLastTargetMetTimestamp(0) { + const std::vector sessionHintRange{ndk::enum_range().begin(), + ndk::enum_range().end()}; + + mLastHintSentTimestamp = std::vector(sessionHintRange.size(), 0); +} APerformanceHintSession::~APerformanceHintSession() { binder::Status ret = mHintSession->close(); @@ -224,10 +239,16 @@ int APerformanceHintSession::reportActualWorkDuration(int64_t actualDurationNano } int APerformanceHintSession::sendHint(int32_t hint) { - if (hint < 0) { - ALOGE("%s: session hint value must be greater than zero", __FUNCTION__); + if (hint < 0 || hint >= static_cast(mLastHintSentTimestamp.size())) { + ALOGE("%s: invalid session hint %d", __FUNCTION__, hint); return EINVAL; } + int64_t now = elapsedRealtimeNano(); + + // Limit sendHint to a pre-detemined rate for safety + if (now < (mLastHintSentTimestamp[hint] + SEND_HINT_TIMEOUT)) { + return 0; + } binder::Status ret = mHintSession->sendHint(hint); @@ -235,6 +256,7 @@ int APerformanceHintSession::sendHint(int32_t hint) { ALOGE("%s: HintSession sendHint failed: %s", __FUNCTION__, ret.exceptionMessage().c_str()); return EPIPE; } + mLastHintSentTimestamp[hint] = now; return 0; } diff --git a/native/android/tests/performance_hint/PerformanceHintNativeTest.cpp b/native/android/tests/performance_hint/PerformanceHintNativeTest.cpp index 1881e60b0f16d..0c2d3b6cd201d 100644 --- a/native/android/tests/performance_hint/PerformanceHintNativeTest.cpp +++ b/native/android/tests/performance_hint/PerformanceHintNativeTest.cpp @@ -122,9 +122,16 @@ TEST_F(PerformanceHintTest, TestSession) { result = APerformanceHint_reportActualWorkDuration(session, -1L); EXPECT_EQ(EINVAL, result); - // Send both valid and invalid session hints int hintId = 2; - EXPECT_CALL(*iSession, sendHint(Eq(2))).Times(Exactly(1)); + EXPECT_CALL(*iSession, sendHint(Eq(hintId))).Times(Exactly(1)); + result = APerformanceHint_sendHint(session, hintId); + EXPECT_EQ(0, result); + usleep(110000); // Sleep for longer than the update timeout. + EXPECT_CALL(*iSession, sendHint(Eq(hintId))).Times(Exactly(1)); + result = APerformanceHint_sendHint(session, hintId); + EXPECT_EQ(0, result); + // Expect to get rate limited if we try to send faster than the limiter allows + EXPECT_CALL(*iSession, sendHint(Eq(hintId))).Times(Exactly(0)); result = APerformanceHint_sendHint(session, hintId); EXPECT_EQ(0, result);