Cache results from Vibrator HAL getters in wrapper
Cache HAL capabilities and supported effects, since these values do not
change for the same underlying HAL.
Bug: 153418251
Test: atest libvibratorservice_test
Change-Id: I23b9480d23810debb065794071319003360857cb
diff --git a/.gitignore b/.gitignore
index 0d20b64..685e379 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1 +1,3 @@
+*.iml
*.pyc
+.idea/
diff --git a/services/vibratorservice/VibratorHalWrapper.cpp b/services/vibratorservice/VibratorHalWrapper.cpp
index 04a8e6a..db27bd9 100644
--- a/services/vibratorservice/VibratorHalWrapper.cpp
+++ b/services/vibratorservice/VibratorHalWrapper.cpp
@@ -44,6 +44,18 @@
// -------------------------------------------------------------------------------------------------
template <class T>
+HalResult<T> loadCached(const std::function<HalResult<T>()>& loadFn, std::optional<T>& cache) {
+ if (cache.has_value()) {
+ return HalResult<T>::ok(cache.value());
+ }
+ HalResult<T> ret = loadFn();
+ if (ret.isOk()) {
+ cache.emplace(ret.value());
+ }
+ return ret;
+}
+
+template <class T>
bool isStaticCastValid(Effect effect) {
T castEffect = static_cast<T>(effect);
auto iter = hardware::hidl_enum_range<T>();
@@ -214,15 +226,23 @@
}
HalResult<Capabilities> AidlHalWrapper::getCapabilities() {
- int32_t capabilities = 0;
- auto result = mHandle->getCapabilities(&capabilities);
- return HalResult<Capabilities>::fromStatus(result, static_cast<Capabilities>(capabilities));
+ std::lock_guard<std::mutex> lock(mCapabilitiesMutex);
+ static auto loadFn = [this]() {
+ int32_t capabilities = 0;
+ auto result = mHandle->getCapabilities(&capabilities);
+ return HalResult<Capabilities>::fromStatus(result, static_cast<Capabilities>(capabilities));
+ };
+ return loadCached<Capabilities>(loadFn, mCapabilities);
}
HalResult<std::vector<Effect>> AidlHalWrapper::getSupportedEffects() {
- std::vector<Effect> supportedEffects;
- auto result = mHandle->getSupportedEffects(&supportedEffects);
- return HalResult<std::vector<Effect>>::fromStatus(result, supportedEffects);
+ std::lock_guard<std::mutex> lock(mSupportedEffectsMutex);
+ static auto loadFn = [this]() {
+ std::vector<Effect> supportedEffects;
+ auto result = mHandle->getSupportedEffects(&supportedEffects);
+ return HalResult<std::vector<Effect>>::fromStatus(result, supportedEffects);
+ };
+ return loadCached<std::vector<Effect>>(loadFn, mSupportedEffects);
}
HalResult<milliseconds> AidlHalWrapper::performEffect(
@@ -279,10 +299,9 @@
}
HalResult<Capabilities> HidlHalWrapperV1_0::getCapabilities() {
- hardware::Return<bool> result = mHandleV1_0->supportsAmplitudeControl();
- Capabilities capabilities =
- result.withDefault(false) ? Capabilities::AMPLITUDE_CONTROL : Capabilities::NONE;
- return HalResult<Capabilities>::fromReturn(result, capabilities);
+ std::lock_guard<std::mutex> lock(mCapabilitiesMutex);
+ return loadCached<Capabilities>(std::bind(&HidlHalWrapperV1_0::getCapabilitiesInternal, this),
+ mCapabilities);
}
HalResult<std::vector<Effect>> HidlHalWrapperV1_0::getSupportedEffects() {
@@ -308,6 +327,13 @@
return HalResult<void>::unsupported();
}
+HalResult<Capabilities> HidlHalWrapperV1_0::getCapabilitiesInternal() {
+ hardware::Return<bool> result = mHandleV1_0->supportsAmplitudeControl();
+ Capabilities capabilities =
+ result.withDefault(false) ? Capabilities::AMPLITUDE_CONTROL : Capabilities::NONE;
+ return HalResult<Capabilities>::fromReturn(result, capabilities);
+}
+
// -------------------------------------------------------------------------------------------------
HalResult<milliseconds> HidlHalWrapperV1_1::performEffect(Effect effect, EffectStrength strength,
@@ -355,19 +381,6 @@
return HalResult<void>::fromStatus(result.withDefault(V1_0::Status::UNKNOWN_ERROR));
}
-HalResult<Capabilities> HidlHalWrapperV1_3::getCapabilities() {
- HalResult<Capabilities> parentResult = HidlHalWrapperV1_2::getCapabilities();
- if (!parentResult.isOk()) {
- // Loading for versions up to v1.2 already failed, so propagate failure.
- return parentResult;
- }
-
- Capabilities capabilities = parentResult.value();
- auto result = mHandleV1_3->supportsExternalControl();
- capabilities |= result.withDefault(false) ? Capabilities::EXTERNAL_CONTROL : Capabilities::NONE;
- return HalResult<Capabilities>::fromReturn(result, capabilities);
-}
-
HalResult<milliseconds> HidlHalWrapperV1_3::performEffect(Effect effect, EffectStrength strength,
const std::function<void()>&) {
if (isStaticCastValid<V1_0::Effect>(effect)) {
@@ -392,6 +405,19 @@
return HalResult<milliseconds>::unsupported();
}
+HalResult<Capabilities> HidlHalWrapperV1_3::getCapabilitiesInternal() {
+ HalResult<Capabilities> parentResult = HidlHalWrapperV1_2::getCapabilitiesInternal();
+ if (!parentResult.isOk()) {
+ // Loading for previous HAL versions already failed, so propagate failure.
+ return parentResult;
+ }
+
+ Capabilities capabilities = parentResult.value();
+ auto result = mHandleV1_3->supportsExternalControl();
+ capabilities |= result.withDefault(false) ? Capabilities::EXTERNAL_CONTROL : Capabilities::NONE;
+ return HalResult<Capabilities>::fromReturn(result, capabilities);
+}
+
// -------------------------------------------------------------------------------------------------
}; // namespace vibrator
diff --git a/services/vibratorservice/include/vibratorservice/VibratorHalWrapper.h b/services/vibratorservice/include/vibratorservice/VibratorHalWrapper.h
index 3d56c43..1a1f64b 100644
--- a/services/vibratorservice/include/vibratorservice/VibratorHalWrapper.h
+++ b/services/vibratorservice/include/vibratorservice/VibratorHalWrapper.h
@@ -181,6 +181,11 @@
private:
const sp<hardware::vibrator::IVibrator> mHandle;
+ std::mutex mCapabilitiesMutex;
+ std::mutex mSupportedEffectsMutex;
+ std::optional<Capabilities> mCapabilities GUARDED_BY(mCapabilitiesMutex);
+ std::optional<std::vector<hardware::vibrator::Effect>> mSupportedEffects
+ GUARDED_BY(mSupportedEffectsMutex);
};
// Wrapper for the HDIL Vibrator HAL v1.0.
@@ -215,6 +220,11 @@
protected:
const sp<hardware::vibrator::V1_0::IVibrator> mHandleV1_0;
+ std::mutex mCapabilitiesMutex;
+ std::optional<Capabilities> mCapabilities GUARDED_BY(mCapabilitiesMutex);
+
+ // Loads directly from IVibrator handle, skipping the mCapabilities cache.
+ virtual HalResult<Capabilities> getCapabilitiesInternal();
};
// Wrapper for the HDIL Vibrator HAL v1.1.
@@ -255,7 +265,6 @@
mHandleV1_3(hardware::vibrator::V1_3::IVibrator::castFrom(handleV1_0)) {}
virtual HalResult<void> setExternalControl(bool enabled) override;
- virtual HalResult<Capabilities> getCapabilities() override;
virtual HalResult<std::chrono::milliseconds> performEffect(
hardware::vibrator::Effect effect, hardware::vibrator::EffectStrength strength,
@@ -263,6 +272,8 @@
protected:
const sp<hardware::vibrator::V1_3::IVibrator> mHandleV1_3;
+
+ virtual HalResult<Capabilities> getCapabilitiesInternal() override;
};
// -------------------------------------------------------------------------------------------------
diff --git a/services/vibratorservice/test/VibratorHalWrapperAidlTest.cpp b/services/vibratorservice/test/VibratorHalWrapperAidlTest.cpp
index a4cdfae..6db449a 100644
--- a/services/vibratorservice/test/VibratorHalWrapperAidlTest.cpp
+++ b/services/vibratorservice/test/VibratorHalWrapperAidlTest.cpp
@@ -22,6 +22,7 @@
#include <gtest/gtest.h>
#include <utils/Log.h>
+#include <thread>
#include <vibratorservice/VibratorHalWrapper.h>
@@ -243,38 +244,76 @@
ASSERT_TRUE(mWrapper->alwaysOnDisable(3).isFailed());
}
-TEST_F(VibratorHalWrapperAidlTest, TestGetCapabilities) {
+TEST_F(VibratorHalWrapperAidlTest, TestGetCapabilitiesDoesNotCacheFailedResult) {
EXPECT_CALL(*mMockHal.get(), getCapabilities(_))
.Times(Exactly(3))
- .WillOnce(DoAll(SetArgPointee<0>(IVibrator::CAP_ON_CALLBACK), Return(Status())))
.WillOnce(
Return(Status::fromExceptionCode(Status::Exception::EX_UNSUPPORTED_OPERATION)))
- .WillRepeatedly(Return(Status::fromExceptionCode(Status::Exception::EX_SECURITY)));
+ .WillOnce(Return(Status::fromExceptionCode(Status::Exception::EX_SECURITY)))
+ .WillRepeatedly(DoAll(SetArgPointee<0>(IVibrator::CAP_ON_CALLBACK), Return(Status())));
+
+ ASSERT_TRUE(mWrapper->getCapabilities().isUnsupported());
+ ASSERT_TRUE(mWrapper->getCapabilities().isFailed());
auto result = mWrapper->getCapabilities();
ASSERT_TRUE(result.isOk());
ASSERT_EQ(vibrator::Capabilities::ON_CALLBACK, result.value());
- ASSERT_TRUE(mWrapper->getCapabilities().isUnsupported());
- ASSERT_TRUE(mWrapper->getCapabilities().isFailed());
}
-TEST_F(VibratorHalWrapperAidlTest, TestGetSupportedEffects) {
+TEST_F(VibratorHalWrapperAidlTest, TestGetCapabilitiesCachesResult) {
+ EXPECT_CALL(*mMockHal.get(), getCapabilities(_))
+ .Times(Exactly(1))
+ .WillRepeatedly(DoAll(SetArgPointee<0>(IVibrator::CAP_ON_CALLBACK), Return(Status())));
+
+ std::vector<std::thread> threads;
+ for (int i = 0; i < 10; i++) {
+ threads.push_back(std::thread([&]() {
+ auto result = mWrapper->getCapabilities();
+ ASSERT_TRUE(result.isOk());
+ ASSERT_EQ(vibrator::Capabilities::ON_CALLBACK, result.value());
+ }));
+ }
+ std::for_each(threads.begin(), threads.end(), [](std::thread& t) { t.join(); });
+}
+
+TEST_F(VibratorHalWrapperAidlTest, TestGetSupportedEffectsDoesNotCacheFailedResult) {
std::vector<Effect> supportedEffects;
supportedEffects.push_back(Effect::CLICK);
supportedEffects.push_back(Effect::TICK);
EXPECT_CALL(*mMockHal.get(), getSupportedEffects(_))
.Times(Exactly(3))
- .WillOnce(DoAll(SetArgPointee<0>(supportedEffects), Return(Status())))
.WillOnce(
Return(Status::fromExceptionCode(Status::Exception::EX_UNSUPPORTED_OPERATION)))
- .WillRepeatedly(Return(Status::fromExceptionCode(Status::Exception::EX_SECURITY)));
+ .WillOnce(Return(Status::fromExceptionCode(Status::Exception::EX_SECURITY)))
+ .WillRepeatedly(DoAll(SetArgPointee<0>(supportedEffects), Return(Status())));
+
+ ASSERT_TRUE(mWrapper->getSupportedEffects().isUnsupported());
+ ASSERT_TRUE(mWrapper->getSupportedEffects().isFailed());
auto result = mWrapper->getSupportedEffects();
ASSERT_TRUE(result.isOk());
ASSERT_EQ(supportedEffects, result.value());
- ASSERT_TRUE(mWrapper->getSupportedEffects().isUnsupported());
- ASSERT_TRUE(mWrapper->getSupportedEffects().isFailed());
+}
+
+TEST_F(VibratorHalWrapperAidlTest, TestGetSupportedEffectsCachesResult) {
+ std::vector<Effect> supportedEffects;
+ supportedEffects.push_back(Effect::CLICK);
+ supportedEffects.push_back(Effect::TICK);
+
+ EXPECT_CALL(*mMockHal.get(), getSupportedEffects(_))
+ .Times(Exactly(1))
+ .WillRepeatedly(DoAll(SetArgPointee<0>(supportedEffects), Return(Status())));
+
+ std::vector<std::thread> threads;
+ for (int i = 0; i < 10; i++) {
+ threads.push_back(std::thread([&]() {
+ auto result = mWrapper->getSupportedEffects();
+ ASSERT_TRUE(result.isOk());
+ ASSERT_EQ(supportedEffects, result.value());
+ }));
+ }
+ std::for_each(threads.begin(), threads.end(), [](std::thread& t) { t.join(); });
}
TEST_F(VibratorHalWrapperAidlTest, TestPerformEffect) {
diff --git a/services/vibratorservice/test/VibratorHalWrapperHidlV1_0Test.cpp b/services/vibratorservice/test/VibratorHalWrapperHidlV1_0Test.cpp
index bc1d14d..7f1016f 100644
--- a/services/vibratorservice/test/VibratorHalWrapperHidlV1_0Test.cpp
+++ b/services/vibratorservice/test/VibratorHalWrapperHidlV1_0Test.cpp
@@ -22,6 +22,7 @@
#include <gtest/gtest.h>
#include <utils/Log.h>
+#include <thread>
#include <vibratorservice/VibratorHalWrapper.h>
@@ -186,29 +187,48 @@
ASSERT_TRUE(mWrapper->alwaysOnDisable(1).isUnsupported());
}
-TEST_F(VibratorHalWrapperHidlV1_0Test, TestGetCapabilities) {
+TEST_F(VibratorHalWrapperHidlV1_0Test, TestGetCapabilitiesDoesNotCacheFailedResult) {
EXPECT_CALL(*mMockHal.get(), supportsAmplitudeControl())
- .Times(Exactly(3))
- .WillOnce([]() { return hardware::Return<bool>(true); })
- .WillOnce([]() { return hardware::Return<bool>(false); })
- .WillRepeatedly([]() {
+ .Times(Exactly(2))
+ .WillOnce([]() {
return hardware::Return<bool>(hardware::Status::fromExceptionCode(-1));
- });
+ })
+ .WillRepeatedly([]() { return hardware::Return<bool>(true); });
- // Amplitude control enabled.
+ ASSERT_TRUE(mWrapper->getCapabilities().isFailed());
+
auto result = mWrapper->getCapabilities();
ASSERT_TRUE(result.isOk());
ASSERT_EQ(vibrator::Capabilities::AMPLITUDE_CONTROL, result.value());
-
- // Amplitude control disabled.
- result = mWrapper->getCapabilities();
- ASSERT_TRUE(result.isOk());
- ASSERT_EQ(vibrator::Capabilities::NONE, result.value());
-
- ASSERT_TRUE(mWrapper->getCapabilities().isFailed());
}
-TEST_F(VibratorHalWrapperHidlV1_0Test, TestGetSupportedEffects) {
+TEST_F(VibratorHalWrapperHidlV1_0Test, TestGetCapabilitiesWithoutAmplitudeControl) {
+ EXPECT_CALL(*mMockHal.get(), supportsAmplitudeControl()).Times(Exactly(1)).WillRepeatedly([]() {
+ return hardware::Return<bool>(false);
+ });
+
+ auto result = mWrapper->getCapabilities();
+ ASSERT_TRUE(result.isOk());
+ ASSERT_EQ(vibrator::Capabilities::NONE, result.value());
+}
+
+TEST_F(VibratorHalWrapperHidlV1_0Test, TestGetCapabilitiesCachesResult) {
+ EXPECT_CALL(*mMockHal.get(), supportsAmplitudeControl()).Times(Exactly(1)).WillRepeatedly([]() {
+ return hardware::Return<bool>(true);
+ });
+
+ std::vector<std::thread> threads;
+ for (int i = 0; i < 10; i++) {
+ threads.push_back(std::thread([&]() {
+ auto result = mWrapper->getCapabilities();
+ ASSERT_TRUE(result.isOk());
+ ASSERT_EQ(vibrator::Capabilities::AMPLITUDE_CONTROL, result.value());
+ }));
+ }
+ std::for_each(threads.begin(), threads.end(), [](std::thread& t) { t.join(); });
+}
+
+TEST_F(VibratorHalWrapperHidlV1_0Test, TestGetSupportedEffectsUnsupported) {
ASSERT_TRUE(mWrapper->getSupportedEffects().isUnsupported());
}
diff --git a/services/vibratorservice/test/VibratorHalWrapperHidlV1_3Test.cpp b/services/vibratorservice/test/VibratorHalWrapperHidlV1_3Test.cpp
index f44c6d8..a799257 100644
--- a/services/vibratorservice/test/VibratorHalWrapperHidlV1_3Test.cpp
+++ b/services/vibratorservice/test/VibratorHalWrapperHidlV1_3Test.cpp
@@ -22,6 +22,7 @@
#include <gtest/gtest.h>
#include <utils/Log.h>
+#include <thread>
#include <vibratorservice/VibratorHalWrapper.h>
@@ -110,21 +111,49 @@
EXPECT_CALL(*mMockHal.get(), supportsExternalControl()).Times(Exactly(1)).WillOnce([]() {
return hardware::Return<bool>(true);
});
+ }
- EXPECT_CALL(*mMockHal.get(), supportsAmplitudeControl()).Times(Exactly(1)).WillOnce([]() {
- return hardware::Return<bool>(false);
- });
- EXPECT_CALL(*mMockHal.get(), supportsExternalControl()).Times(Exactly(1)).WillOnce([]() {
- return hardware::Return<bool>(true);
- });
+ auto result = mWrapper->getCapabilities();
+ ASSERT_TRUE(result.isOk());
+ ASSERT_EQ(vibrator::Capabilities::AMPLITUDE_CONTROL | vibrator::Capabilities::EXTERNAL_CONTROL,
+ result.value());
+}
+TEST_F(VibratorHalWrapperHidlV1_3Test, TestGetCapabilitiesOnlyAmplitudeControl) {
+ {
+ InSequence seq;
EXPECT_CALL(*mMockHal.get(), supportsAmplitudeControl()).Times(Exactly(1)).WillOnce([]() {
return hardware::Return<bool>(true);
});
EXPECT_CALL(*mMockHal.get(), supportsExternalControl()).Times(Exactly(1)).WillOnce([]() {
return hardware::Return<bool>(false);
});
+ }
+ auto result = mWrapper->getCapabilities();
+ ASSERT_TRUE(result.isOk());
+ ASSERT_EQ(vibrator::Capabilities::AMPLITUDE_CONTROL, result.value());
+}
+
+TEST_F(VibratorHalWrapperHidlV1_3Test, TestGetCapabilitiesOnlyExternalControl) {
+ {
+ InSequence seq;
+ EXPECT_CALL(*mMockHal.get(), supportsAmplitudeControl()).Times(Exactly(1)).WillOnce([]() {
+ return hardware::Return<bool>(false);
+ });
+ EXPECT_CALL(*mMockHal.get(), supportsExternalControl()).Times(Exactly(1)).WillOnce([]() {
+ return hardware::Return<bool>(true);
+ });
+ }
+
+ auto result = mWrapper->getCapabilities();
+ ASSERT_TRUE(result.isOk());
+ ASSERT_EQ(vibrator::Capabilities::EXTERNAL_CONTROL, result.value());
+}
+
+TEST_F(VibratorHalWrapperHidlV1_3Test, TestGetCapabilitiesNone) {
+ {
+ InSequence seq;
EXPECT_CALL(*mMockHal.get(), supportsAmplitudeControl())
.Times(Exactly(1))
.WillRepeatedly([]() { return hardware::Return<bool>(false); });
@@ -133,25 +162,8 @@
});
}
- // Both enabled.
auto result = mWrapper->getCapabilities();
ASSERT_TRUE(result.isOk());
- ASSERT_EQ(vibrator::Capabilities::AMPLITUDE_CONTROL | vibrator::Capabilities::EXTERNAL_CONTROL,
- result.value());
-
- // Amplitude control disabled.
- result = mWrapper->getCapabilities();
- ASSERT_TRUE(result.isOk());
- ASSERT_EQ(vibrator::Capabilities::EXTERNAL_CONTROL, result.value());
-
- // External control disabled.
- result = mWrapper->getCapabilities();
- ASSERT_TRUE(result.isOk());
- ASSERT_EQ(vibrator::Capabilities::AMPLITUDE_CONTROL, result.value());
-
- // Both disabled.
- result = mWrapper->getCapabilities();
- ASSERT_TRUE(result.isOk());
ASSERT_EQ(vibrator::Capabilities::NONE, result.value());
}
@@ -178,6 +190,73 @@
ASSERT_TRUE(mWrapper->getCapabilities().isFailed());
}
+TEST_F(VibratorHalWrapperHidlV1_3Test, TestGetCapabilitiesCachesResult) {
+ {
+ InSequence seq;
+ EXPECT_CALL(*mMockHal.get(), supportsAmplitudeControl())
+ .Times(Exactly(1))
+ .WillRepeatedly([]() { return hardware::Return<bool>(true); });
+ EXPECT_CALL(*mMockHal.get(), supportsExternalControl()).Times(Exactly(1)).WillOnce([]() {
+ return hardware::Return<bool>(false);
+ });
+ }
+
+ std::vector<std::thread> threads;
+ for (int i = 0; i < 10; i++) {
+ threads.push_back(std::thread([&]() {
+ auto result = mWrapper->getCapabilities();
+ ASSERT_TRUE(result.isOk());
+ ASSERT_EQ(vibrator::Capabilities::AMPLITUDE_CONTROL, result.value());
+ }));
+ }
+ std::for_each(threads.begin(), threads.end(), [](std::thread& t) { t.join(); });
+}
+
+TEST_F(VibratorHalWrapperHidlV1_3Test, TestGetCapabilitiesDoesNotCacheFailedResult) {
+ {
+ InSequence seq;
+ EXPECT_CALL(*mMockHal.get(), supportsAmplitudeControl())
+ .Times(Exactly(1))
+ .WillRepeatedly([]() {
+ return hardware::Return<bool>(hardware::Status::fromExceptionCode(-1));
+ });
+
+ EXPECT_CALL(*mMockHal.get(), supportsAmplitudeControl())
+ .Times(Exactly(1))
+ .WillRepeatedly([]() { return hardware::Return<bool>(true); });
+ EXPECT_CALL(*mMockHal.get(), supportsExternalControl())
+ .Times(Exactly(1))
+ .WillRepeatedly([]() {
+ return hardware::Return<bool>(hardware::Status::fromExceptionCode(-1));
+ });
+
+ EXPECT_CALL(*mMockHal.get(), supportsAmplitudeControl())
+ .Times(Exactly(1))
+ .WillRepeatedly([]() { return hardware::Return<bool>(true); });
+ EXPECT_CALL(*mMockHal.get(), supportsExternalControl())
+ .Times(Exactly(1))
+ .WillRepeatedly([]() { return hardware::Return<bool>(false); });
+ }
+
+ // Call to supportsAmplitudeControl failed.
+ auto result = mWrapper->getCapabilities();
+ ASSERT_TRUE(result.isFailed());
+
+ // Call to supportsExternalControl failed.
+ result = mWrapper->getCapabilities();
+ ASSERT_TRUE(result.isFailed());
+
+ // Returns successful result from third call.
+ result = mWrapper->getCapabilities();
+ ASSERT_TRUE(result.isOk());
+ ASSERT_EQ(vibrator::Capabilities::AMPLITUDE_CONTROL, result.value());
+
+ // Returns cached successful result.
+ result = mWrapper->getCapabilities();
+ ASSERT_TRUE(result.isOk());
+ ASSERT_EQ(vibrator::Capabilities::AMPLITUDE_CONTROL, result.value());
+}
+
TEST_F(VibratorHalWrapperHidlV1_3Test, TestPerformEffectV1_0) {
EXPECT_CALL(*mMockHal.get(),
perform(Eq(V1_0::Effect::CLICK), Eq(V1_0::EffectStrength::LIGHT), _))