Remove lock for fakeVehicleHardware callbacks.
The callbacks are called very frequently, guarding them with the
main mLock will cause the mLock to become very hot and possibly
blocking other operations from owning the lock. Since the callback
is only set once by DefaultVehicleHal during initialization, it
is really not necessary to guard them with lock.
Test: manual run on gcar_emu
Bug: 255574557
Change-Id: Icc1f90b89578a27729ef8beae3a475966f72d318
diff --git a/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h b/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h
index 1636cb6..e515bad 100644
--- a/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h
+++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h
@@ -138,9 +138,12 @@
std::unique_ptr<RecurrentTimer> mRecurrentTimer;
// GeneratorHub is thread-safe.
std::unique_ptr<GeneratorHub> mGeneratorHub;
+
+ // Only allowed to set once.
+ std::unique_ptr<const PropertyChangeCallback> mOnPropertyChangeCallback;
+ std::unique_ptr<const PropertySetErrorCallback> mOnPropertySetErrorCallback;
+
std::mutex mLock;
- std::unique_ptr<const PropertyChangeCallback> mOnPropertyChangeCallback GUARDED_BY(mLock);
- std::unique_ptr<const PropertySetErrorCallback> mOnPropertySetErrorCallback GUARDED_BY(mLock);
std::unordered_map<PropIdAreaId, std::shared_ptr<RecurrentTimer::Callback>, PropIdAreaIdHash>
mRecurrentActions GUARDED_BY(mLock);
std::unordered_map<PropIdAreaId, VehiclePropValuePool::RecyclableType, PropIdAreaIdHash>
diff --git a/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp b/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp
index 736ecaa..dd76524 100644
--- a/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp
+++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp
@@ -1228,13 +1228,19 @@
void FakeVehicleHardware::registerOnPropertyChangeEvent(
std::unique_ptr<const PropertyChangeCallback> callback) {
- std::scoped_lock<std::mutex> lockGuard(mLock);
+ if (mOnPropertyChangeCallback != nullptr) {
+ ALOGE("registerOnPropertyChangeEvent must only be called once");
+ return;
+ }
mOnPropertyChangeCallback = std::move(callback);
}
void FakeVehicleHardware::registerOnPropertySetErrorEvent(
std::unique_ptr<const PropertySetErrorCallback> callback) {
- std::scoped_lock<std::mutex> lockGuard(mLock);
+ if (mOnPropertySetErrorCallback != nullptr) {
+ ALOGE("registerOnPropertySetErrorEvent must only be called once");
+ return;
+ }
mOnPropertySetErrorCallback = std::move(callback);
}
@@ -1278,8 +1284,6 @@
}
void FakeVehicleHardware::onValueChangeCallback(const VehiclePropValue& value) {
- std::scoped_lock<std::mutex> lockGuard(mLock);
-
if (mOnPropertyChangeCallback == nullptr) {
return;
}
diff --git a/automotive/vehicle/aidl/impl/hardware/include/IVehicleHardware.h b/automotive/vehicle/aidl/impl/hardware/include/IVehicleHardware.h
index 759db41..d92ccfd 100644
--- a/automotive/vehicle/aidl/impl/hardware/include/IVehicleHardware.h
+++ b/automotive/vehicle/aidl/impl/hardware/include/IVehicleHardware.h
@@ -116,11 +116,12 @@
virtual aidl::android::hardware::automotive::vehicle::StatusCode checkHealth() = 0;
// Register a callback that would be called when there is a property change event from vehicle.
+ // Must only be called once during initialization.
virtual void registerOnPropertyChangeEvent(
std::unique_ptr<const PropertyChangeCallback> callback) = 0;
// Register a callback that would be called when there is a property set error event from
- // vehicle.
+ // vehicle. Must only be called once during initialization.
virtual void registerOnPropertySetErrorEvent(
std::unique_ptr<const PropertySetErrorCallback> callback) = 0;
};