Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3d8595e3 authored by Yu Shan's avatar Yu Shan
Browse files

Use ObjectPool objects in property store. 

Use object pool recyclable objects in property store API to improve
code efficiency. This would reduce the number of unnecssary copy of
property values.
This CL also adds a upateStatus option which by default is set to false,
so property from client would not override the property status.

Test: atest VehicleHalVehicleUtilsTest
Bug: 200737967
Change-Id: Ie96a41ee441e085e10ad1059b67c708ff7173ae0
parent c865a586

automotive/vehicle/aidl/impl/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ cc_defaults { 
    name: "VehicleHalDefaults",

    static_libs: [

        "android.hardware.automotive.vehicle-V1-ndk",

        "libmath",

    ],

    shared_libs: [

        "libbase",

automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h

+26 −20
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ 
#include <unordered_map>



#include <VehicleHalTypes.h>

#include <VehicleObjectPool.h>

#include <android-base/result.h>

#include <android-base/thread_annotations.h>
@@ -41,6 +42,9 @@ namespace vehicle { 
// This class is thread-safe, however it uses blocking synchronization across all methods.

class VehiclePropertyStore {

  public:

    explicit VehiclePropertyStore(std::shared_ptr<VehiclePropValuePool> valuePool)

        : mValuePool(valuePool) {}



    // Function that used to calculate unique token for given VehiclePropValue.

    using TokenFunction = ::std::function<int64_t(

            const ::aidl::android::hardware::automotive::vehicle::VehiclePropValue& value)>;
@@ -53,10 +57,13 @@ class VehiclePropertyStore { 
            const ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig& config,

            TokenFunction tokenFunc = nullptr);



    // Stores provided value. Returns true if value was written returns false if config wasn't

    // registered.

    ::android::base::Result<void> writeValue(

            const ::aidl::android::hardware::automotive::vehicle::VehiclePropValue& propValue);

    // Stores provided value. Returns error if config wasn't registered. If 'updateStatus' is

    // true, the 'status' in 'propValue' would be stored. Otherwise, if this is a new value,

    // 'status' would be initialized to {@code VehiclePropertyStatus::AVAILABLE}, if this is to

    // override an existing value, the status for the existing value would be used for the

    // overridden value.

    ::android::base::Result<void> writeValue(VehiclePropValuePool::RecyclableType propValue,

                                             bool updateStatus = false);



    // Remove a given property value from the property store. The 'propValue' would be used to

    // generate the key for the value to remove.
@@ -67,24 +74,19 @@ class VehiclePropertyStore { 
    void removeValuesForProperty(int32_t propId);



    // Read all the stored values.

    std::vector<::aidl::android::hardware::automotive::vehicle::VehiclePropValue> readAllValues()

            const;

    std::vector<VehiclePropValuePool::RecyclableType> readAllValues() const;



    // Read all the values for the property.

    ::android::base::Result<

            std::vector<::aidl::android::hardware::automotive::vehicle::VehiclePropValue>>

    ::android::base::Result<std::vector<VehiclePropValuePool::RecyclableType>>

    readValuesForProperty(int32_t propId) const;



    // Read the value for the requested property.

    ::android::base::Result<

            std::unique_ptr<::aidl::android::hardware::automotive::vehicle::VehiclePropValue>>

    readValue(

    ::android::base::Result<VehiclePropValuePool::RecyclableType> readValue(

            const ::aidl::android::hardware::automotive::vehicle::VehiclePropValue& request) const;



    // Read the value for the requested property.

    ::android::base::Result<

            std::unique_ptr<::aidl::android::hardware::automotive::vehicle::VehiclePropValue>>

    readValue(int32_t prop, int32_t area = 0, int64_t token = 0) const;

    ::android::base::Result<VehiclePropValuePool::RecyclableType> readValue(

            int32_t prop, int32_t area = 0, int64_t token = 0) const;



    // Get all property configs.

    std::vector<::aidl::android::hardware::automotive::vehicle::VehiclePropConfig> getAllConfigs()
@@ -100,20 +102,25 @@ class VehiclePropertyStore { 
        int32_t area;

        int64_t token;



        std::string toString() const;



        bool operator==(const RecordId& other) const;

        bool operator<(const RecordId& other) const;

    };



        std::string toString() const;

    struct RecordIdHash {

        size_t operator()(RecordId const& recordId) const;

    };



    struct Record {

        ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig propConfig;

        TokenFunction tokenFunction;

        std::map<RecordId, ::aidl::android::hardware::automotive::vehicle::VehiclePropValue> values;

        std::unordered_map<RecordId, VehiclePropValuePool::RecyclableType, RecordIdHash> values;

    };



    mutable std::mutex mLock;

    std::unordered_map<int32_t, Record> mRecordsByPropId GUARDED_BY(mLock);

    // {@code VehiclePropValuePool} is thread-safe.

    std::shared_ptr<VehiclePropValuePool> mValuePool;



    const Record* getRecordLocked(int32_t propId) const;
@@ -123,9 +130,8 @@ class VehiclePropertyStore { 
            const ::aidl::android::hardware::automotive::vehicle::VehiclePropValue& propValue,

            const Record& record) const;



    ::android::base::Result<

            std::unique_ptr<::aidl::android::hardware::automotive::vehicle::VehiclePropValue>>

    readValueLocked(const RecordId& recId, const Record& record) const;

    ::android::base::Result<VehiclePropValuePool::RecyclableType> readValueLocked(

            const RecordId& recId, const Record& record) const;

};



}  // namespace vehicle

automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp

+46 −35
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ 


#include <VehicleUtils.h>

#include <android-base/format.h>

#include <math/HashCombine.h>



namespace android {

namespace hardware {
@@ -29,6 +30,7 @@ namespace vehicle { 


using ::aidl::android::hardware::automotive::vehicle::VehicleAreaConfig;

using ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig;

using ::aidl::android::hardware::automotive::vehicle::VehiclePropertyStatus;

using ::aidl::android::hardware::automotive::vehicle::VehiclePropValue;

using ::android::base::Result;
@@ -36,14 +38,17 @@ bool VehiclePropertyStore::RecordId::operator==(const VehiclePropertyStore::Reco 
    return area == other.area && token == other.token;

}



bool VehiclePropertyStore::RecordId::operator<(const VehiclePropertyStore::RecordId& other) const {

    return area < other.area || (area == other.area && token < other.token);

}



std::string VehiclePropertyStore::RecordId::toString() const {

    return ::fmt::format("RecordID{{.areaId={:d}, .token={:d}}}", area, token);

}



size_t VehiclePropertyStore::RecordIdHash::operator()(RecordId const& recordId) const {

    size_t res = 0;

    hashCombine(res, recordId.area);

    hashCombine(res, recordId.token);

    return res;

}



const VehiclePropertyStore::Record* VehiclePropertyStore::getRecordLocked(int32_t propId) const

        REQUIRES(mLock) {

    auto RecordIt = mRecordsByPropId.find(propId);
@@ -68,13 +73,13 @@ VehiclePropertyStore::RecordId VehiclePropertyStore::getRecordIdLocked( 
    return recId;

}



Result<std::unique_ptr<VehiclePropValue>> VehiclePropertyStore::readValueLocked(

Result<VehiclePropValuePool::RecyclableType> VehiclePropertyStore::readValueLocked(

        const RecordId& recId, const Record& record) const REQUIRES(mLock) {

    auto it = record.values.find(recId);

    if (it == record.values.end()) {

        return Errorf("Record ID: {} is not found", recId.toString());

    }

    return std::make_unique<VehiclePropValue>(it->second);

    return mValuePool->obtain(*(it->second));

}



void VehiclePropertyStore::registerProperty(const VehiclePropConfig& config,
@@ -87,36 +92,42 @@ void VehiclePropertyStore::registerProperty(const VehiclePropConfig& config, 
    };

}



Result<void> VehiclePropertyStore::writeValue(const VehiclePropValue& propValue) {

Result<void> VehiclePropertyStore::writeValue(VehiclePropValuePool::RecyclableType propValue,

                                              bool updateStatus) {

    std::lock_guard<std::mutex> g(mLock);



    VehiclePropertyStore::Record* record = getRecordLocked(propValue.prop);

    VehiclePropertyStore::Record* record = getRecordLocked(propValue->prop);

    if (record == nullptr) {

        return Errorf("property: {:d} not registered", propValue.prop);

        return Errorf("property: {:d} not registered", propValue->prop);

    }



    if (!isGlobalProp(propValue.prop) && getAreaConfig(propValue, record->propConfig) == nullptr) {

        return Errorf("no config for property: {:d} area: {:d}", propValue.prop, propValue.areaId);

    if (!isGlobalProp(propValue->prop) &&

        getAreaConfig(*propValue, record->propConfig) == nullptr) {

        return Errorf("no config for property: {:d} area: {:d}", propValue->prop,

                      propValue->areaId);

    }



    VehiclePropertyStore::RecordId recId = getRecordIdLocked(propValue, *record);

    VehiclePropertyStore::RecordId recId = getRecordIdLocked(*propValue, *record);

    auto it = record->values.find(recId);

    if (it == record->values.end()) {

        record->values[recId] = propValue;

        record->values[recId] = std::move(propValue);

        if (!updateStatus) {

            record->values[recId]->status = VehiclePropertyStatus::AVAILABLE;

        }

        return {};

    }

    VehiclePropValue* valueToUpdate = &(it->second);



    const VehiclePropValue* valueToUpdate = it->second.get();

    long oldTimestamp = valueToUpdate->timestamp;

    VehiclePropertyStatus oldStatus = valueToUpdate->status;

    // propValue is outdated and drops it.

    if (valueToUpdate->timestamp > propValue.timestamp) {

        return Errorf("outdated timestamp: {:d}", propValue.timestamp);

    }

    // Update the propertyValue.

    // The timestamp in propertyStore should only be updated by the server side. It indicates

    // the time when the event is generated by the server.

    valueToUpdate->timestamp = propValue.timestamp;

    valueToUpdate->value = propValue.value;

    valueToUpdate->status = propValue.status;

    if (oldTimestamp > propValue->timestamp) {

        return Errorf("outdated timestamp: {:d}", propValue->timestamp);

    }

    record->values[recId] = std::move(propValue);

    if (!updateStatus) {

        record->values[recId]->status = oldStatus;

    }



    return {};

}
@@ -145,25 +156,25 @@ void VehiclePropertyStore::removeValuesForProperty(int32_t propId) { 
    record->values.clear();

}



std::vector<VehiclePropValue> VehiclePropertyStore::readAllValues() const {

std::vector<VehiclePropValuePool::RecyclableType> VehiclePropertyStore::readAllValues() const {

    std::lock_guard<std::mutex> g(mLock);



    std::vector<VehiclePropValue> allValues;

    std::vector<VehiclePropValuePool::RecyclableType> allValues;



    for (auto const& [_, record] : mRecordsByPropId) {

        for (auto const& [_, value] : record.values) {

            allValues.push_back(value);

            allValues.push_back(std::move(mValuePool->obtain(*value)));

        }

    }



    return allValues;

}



Result<std::vector<VehiclePropValue>> VehiclePropertyStore::readValuesForProperty(

        int32_t propId) const {

Result<std::vector<VehiclePropValuePool::RecyclableType>>

VehiclePropertyStore::readValuesForProperty(int32_t propId) const {

    std::lock_guard<std::mutex> g(mLock);



    std::vector<VehiclePropValue> values;

    std::vector<VehiclePropValuePool::RecyclableType> values;



    const VehiclePropertyStore::Record* record = getRecordLocked(propId);

    if (record == nullptr) {
@@ -171,12 +182,12 @@ Result<std::vector<VehiclePropValue>> VehiclePropertyStore::readValuesForPropert 
    }



    for (auto const& [_, value] : record->values) {

        values.push_back(value);

        values.push_back(std::move(mValuePool->obtain(*value)));

    }

    return values;

}



Result<std::unique_ptr<VehiclePropValue>> VehiclePropertyStore::readValue(

Result<VehiclePropValuePool::RecyclableType> VehiclePropertyStore::readValue(

        const VehiclePropValue& propValue) const {

    std::lock_guard<std::mutex> g(mLock);
@@ -189,7 +200,7 @@ Result<std::unique_ptr<VehiclePropValue>> VehiclePropertyStore::readValue( 
    return readValueLocked(recId, *record);

}



Result<std::unique_ptr<VehiclePropValue>> VehiclePropertyStore::readValue(int32_t propId,

Result<VehiclePropValuePool::RecyclableType> VehiclePropertyStore::readValue(int32_t propId,

                                                                             int32_t areaId,

                                                                             int64_t token) const {

    std::lock_guard<std::mutex> g(mLock);

automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp

+135 −61

File changed.

Preview size limit exceeded, changes collapsed.