Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make ClusterStateCache use less memory when not storing attribute values. #33058

Merged
merged 3 commits into from
Apr 19, 2024
Merged

Make ClusterStateCache use less memory when not storing attribute values. #33058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bf2cae9
Make ClusterStateCache use less memory when not storing attribute val…
bzbarsky-apple Apr 19, 2024
1abc1e1
Address review comment.
bzbarsky-apple Apr 19, 2024
c980863
Actually fix the review comment.
bzbarsky-apple Apr 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
191 changes: 129 additions & 62 deletions src/app/ClusterStateCache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ namespace app {
namespace {

// Determine how much space a StatusIB takes up on the wire.
size_t SizeOfStatusIB(const StatusIB & aStatus)
uint32_t SizeOfStatusIB(const StatusIB & aStatus)
{
// 1 byte: anonymous tag control byte for struct.
// 1 byte: control byte for uint8 value.
// 1 byte: context-specific tag for uint8 value.
// 1 byte: the uint8 value.
// 1 byte: end of container.
size_t size = 5;
uint32_t size = 5;

if (aStatus.mClusterStatus.HasValue())
{
Expand All @@ -49,7 +49,8 @@ size_t SizeOfStatusIB(const StatusIB & aStatus)

} // anonymous namespace

CHIP_ERROR ClusterStateCache::GetElementTLVSize(TLV::TLVReader * apData, size_t & aSize)
template <bool CanEnableDataCaching>
CHIP_ERROR ClusterStateCacheT<CanEnableDataCaching>::GetElementTLVSize(TLV::TLVReader * apData, uint32_t & aSize)
{
Platform::ScopedMemoryBufferWithSize<uint8_t> backingBuffer;
TLV::TLVReader reader;
Expand All @@ -64,8 +65,9 @@ CHIP_ERROR ClusterStateCache::GetElementTLVSize(TLV::TLVReader * apData, size_t
return CHIP_NO_ERROR;
}

CHIP_ERROR ClusterStateCache::UpdateCache(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData,
const StatusIB & aStatus)
template <bool CanEnableDataCaching>
CHIP_ERROR ClusterStateCacheT<CanEnableDataCaching>::UpdateCache(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData,
const StatusIB & aStatus)
{
AttributeState state;
bool endpointIsNew = false;
Expand All @@ -82,24 +84,32 @@ CHIP_ERROR ClusterStateCache::UpdateCache(const ConcreteDataAttributePath & aPat

if (apData)
{
size_t elementSize = 0;
uint32_t elementSize = 0;
ReturnErrorOnFailure(GetElementTLVSize(apData, elementSize));

if (mCacheData)
if constexpr (CanEnableDataCaching)
{
Platform::ScopedMemoryBufferWithSize<uint8_t> backingBuffer;
backingBuffer.Calloc(elementSize);
VerifyOrReturnError(backingBuffer.Get() != nullptr, CHIP_ERROR_NO_MEMORY);
TLV::ScopedBufferTLVWriter writer(std::move(backingBuffer), elementSize);
ReturnErrorOnFailure(writer.CopyElement(TLV::AnonymousTag(), *apData));
ReturnErrorOnFailure(writer.Finalize(backingBuffer));

state.Set<AttributeData>(std::move(backingBuffer));
if (mCacheData)
{
Platform::ScopedMemoryBufferWithSize<uint8_t> backingBuffer;
backingBuffer.Calloc(elementSize);
VerifyOrReturnError(backingBuffer.Get() != nullptr, CHIP_ERROR_NO_MEMORY);
TLV::ScopedBufferTLVWriter writer(std::move(backingBuffer), elementSize);
ReturnErrorOnFailure(writer.CopyElement(TLV::AnonymousTag(), *apData));
ReturnErrorOnFailure(writer.Finalize(backingBuffer));

state.template Set<AttributeData>(std::move(backingBuffer));
}
else
{
state.template Set<uint32_t>(elementSize);
}
}
else
{
state.Set<size_t>(elementSize);
state = elementSize;
}

//
// Clear out the committed data version and only set it again once we have received all data for this cluster.
// Otherwise, we may have incomplete data that looks like it's complete since it has a valid data version.
Expand Down Expand Up @@ -132,13 +142,20 @@ CHIP_ERROR ClusterStateCache::UpdateCache(const ConcreteDataAttributePath & aPat
}
else
{
if (mCacheData)
if constexpr (CanEnableDataCaching)
{
state.Set<StatusIB>(aStatus);
if (mCacheData)
{
state.template Set<StatusIB>(aStatus);
}
else
{
state.template Set<uint32_t>(SizeOfStatusIB(aStatus));
}
}
else
{
state.Set<size_t>(SizeOfStatusIB(aStatus));
state = SizeOfStatusIB(aStatus);
}
}

Expand All @@ -161,7 +178,9 @@ CHIP_ERROR ClusterStateCache::UpdateCache(const ConcreteDataAttributePath & aPat
return CHIP_NO_ERROR;
}

CHIP_ERROR ClusterStateCache::UpdateEventCache(const EventHeader & aEventHeader, TLV::TLVReader * apData, const StatusIB * apStatus)
template <bool CanEnableDataCaching>
CHIP_ERROR ClusterStateCacheT<CanEnableDataCaching>::UpdateEventCache(const EventHeader & aEventHeader, TLV::TLVReader * apData,
const StatusIB * apStatus)
{
if (apData)
{
Expand Down Expand Up @@ -208,15 +227,17 @@ CHIP_ERROR ClusterStateCache::UpdateEventCache(const EventHeader & aEventHeader,
return CHIP_NO_ERROR;
}

void ClusterStateCache::OnReportBegin()
template <bool CanEnableDataCaching>
void ClusterStateCacheT<CanEnableDataCaching>::OnReportBegin()
{
mLastReportDataPath = ConcreteClusterPath(kInvalidEndpointId, kInvalidClusterId);
mChangedAttributeSet.clear();
mAddedEndpoints.clear();
mCallback.OnReportBegin();
}

void ClusterStateCache::CommitPendingDataVersion()
template <bool CanEnableDataCaching>
void ClusterStateCacheT<CanEnableDataCaching>::CommitPendingDataVersion()
{
if (!mLastReportDataPath.IsValidConcreteClusterPath())
{
Expand All @@ -231,7 +252,8 @@ void ClusterStateCache::CommitPendingDataVersion()
}
}

void ClusterStateCache::OnReportEnd()
template <bool CanEnableDataCaching>
void ClusterStateCacheT<CanEnableDataCaching>::OnReportEnd()
{
CommitPendingDataVersion();
mLastReportDataPath = ConcreteClusterPath(kInvalidEndpointId, kInvalidClusterId);
Expand Down Expand Up @@ -260,26 +282,35 @@ void ClusterStateCache::OnReportEnd()
mCallback.OnReportEnd();
}

CHIP_ERROR ClusterStateCache::Get(const ConcreteAttributePath & path, TLV::TLVReader & reader) const
template <>
CHIP_ERROR ClusterStateCacheT<true>::Get(const ConcreteAttributePath & path, TLV::TLVReader & reader) const
{
CHIP_ERROR err;
auto attributeState = GetAttributeState(path.mEndpointId, path.mClusterId, path.mAttributeId, err);
ReturnErrorOnFailure(err);
if (attributeState->Is<StatusIB>())

if (attributeState->template Is<StatusIB>())
{
return CHIP_ERROR_IM_STATUS_CODE_RECEIVED;
}

if (!attributeState->Is<AttributeData>())
if (!attributeState->template Is<AttributeData>())
{
return CHIP_ERROR_KEY_NOT_FOUND;
}

reader.Init(attributeState->Get<AttributeData>().Get(), attributeState->Get<AttributeData>().AllocatedSize());
reader.Init(attributeState->template Get<AttributeData>().Get(), attributeState->template Get<AttributeData>().AllocatedSize());
return reader.Next();
}

CHIP_ERROR ClusterStateCache::Get(EventNumber eventNumber, TLV::TLVReader & reader) const
template <>
CHIP_ERROR ClusterStateCacheT<false>::Get(const ConcreteAttributePath & path, TLV::TLVReader & reader) const
{
return CHIP_ERROR_KEY_NOT_FOUND;
}

template <bool CanEnableDataCaching>
CHIP_ERROR ClusterStateCacheT<CanEnableDataCaching>::Get(EventNumber eventNumber, TLV::TLVReader & reader) const
{
CHIP_ERROR err;

Expand All @@ -295,7 +326,9 @@ CHIP_ERROR ClusterStateCache::Get(EventNumber eventNumber, TLV::TLVReader & read
return CHIP_NO_ERROR;
}

const ClusterStateCache::EndpointState * ClusterStateCache::GetEndpointState(EndpointId endpointId, CHIP_ERROR & err) const
template <bool CanEnableDataCaching>
const typename ClusterStateCacheT<CanEnableDataCaching>::EndpointState *
ClusterStateCacheT<CanEnableDataCaching>::GetEndpointState(EndpointId endpointId, CHIP_ERROR & err) const
{
auto endpointIter = mCache.find(endpointId);
if (endpointIter == mCache.end())
Expand All @@ -308,8 +341,9 @@ const ClusterStateCache::EndpointState * ClusterStateCache::GetEndpointState(End
return &endpointIter->second;
}

const ClusterStateCache::ClusterState * ClusterStateCache::GetClusterState(EndpointId endpointId, ClusterId clusterId,
CHIP_ERROR & err) const
template <bool CanEnableDataCaching>
const typename ClusterStateCacheT<CanEnableDataCaching>::ClusterState *
ClusterStateCacheT<CanEnableDataCaching>::GetClusterState(EndpointId endpointId, ClusterId clusterId, CHIP_ERROR & err) const
{
auto endpointState = GetEndpointState(endpointId, err);
if (err != CHIP_NO_ERROR)
Expand All @@ -328,8 +362,10 @@ const ClusterStateCache::ClusterState * ClusterStateCache::GetClusterState(Endpo
return &clusterState->second;
}

const ClusterStateCache::AttributeState * ClusterStateCache::GetAttributeState(EndpointId endpointId, ClusterId clusterId,
AttributeId attributeId, CHIP_ERROR & err) const
template <bool CanEnableDataCaching>
const typename ClusterStateCacheT<CanEnableDataCaching>::AttributeState *
ClusterStateCacheT<CanEnableDataCaching>::GetAttributeState(EndpointId endpointId, ClusterId clusterId, AttributeId attributeId,
CHIP_ERROR & err) const
{
auto clusterState = GetClusterState(endpointId, clusterId, err);
if (err != CHIP_NO_ERROR)
Expand All @@ -348,7 +384,9 @@ const ClusterStateCache::AttributeState * ClusterStateCache::GetAttributeState(E
return &attributeState->second;
}

const ClusterStateCache::EventData * ClusterStateCache::GetEventData(EventNumber eventNumber, CHIP_ERROR & err) const
template <bool CanEnableDataCaching>
const typename ClusterStateCacheT<CanEnableDataCaching>::EventData *
ClusterStateCacheT<CanEnableDataCaching>::GetEventData(EventNumber eventNumber, CHIP_ERROR & err) const
{
EventData compareKey;

Expand All @@ -364,7 +402,9 @@ const ClusterStateCache::EventData * ClusterStateCache::GetEventData(EventNumber
return &(*eventData);
}

void ClusterStateCache::OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const StatusIB & aStatus)
template <bool CanEnableDataCaching>
void ClusterStateCacheT<CanEnableDataCaching>::OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData,
const StatusIB & aStatus)
{
//
// Since the cache itself is a ReadClient::Callback, it may be incorrectly passed in directly when registering with the
Expand Down Expand Up @@ -394,7 +434,9 @@ void ClusterStateCache::OnAttributeData(const ConcreteDataAttributePath & aPath,
mCallback.OnAttributeData(aPath, apData ? &dataSnapshot : nullptr, aStatus);
}

CHIP_ERROR ClusterStateCache::GetVersion(const ConcreteClusterPath & aPath, Optional<DataVersion> & aVersion) const
template <bool CanEnableDataCaching>
CHIP_ERROR ClusterStateCacheT<CanEnableDataCaching>::GetVersion(const ConcreteClusterPath & aPath,
Optional<DataVersion> & aVersion) const
{
VerifyOrReturnError(aPath.IsValidConcreteClusterPath(), CHIP_ERROR_INVALID_ARGUMENT);
CHIP_ERROR err;
Expand All @@ -404,7 +446,9 @@ CHIP_ERROR ClusterStateCache::GetVersion(const ConcreteClusterPath & aPath, Opti
return CHIP_NO_ERROR;
}

void ClusterStateCache::OnEventData(const EventHeader & aEventHeader, TLV::TLVReader * apData, const StatusIB * apStatus)
template <bool CanEnableDataCaching>
void ClusterStateCacheT<CanEnableDataCaching>::OnEventData(const EventHeader & aEventHeader, TLV::TLVReader * apData,
const StatusIB * apStatus)
{
VerifyOrDie(apData != nullptr || apStatus != nullptr);

Expand All @@ -418,23 +462,31 @@ void ClusterStateCache::OnEventData(const EventHeader & aEventHeader, TLV::TLVRe
mCallback.OnEventData(aEventHeader, apData ? &dataSnapshot : nullptr, apStatus);
}

CHIP_ERROR ClusterStateCache::GetStatus(const ConcreteAttributePath & path, StatusIB & status) const
template <>
CHIP_ERROR ClusterStateCacheT<true>::GetStatus(const ConcreteAttributePath & path, StatusIB & status) const
{
CHIP_ERROR err;

auto attributeState = GetAttributeState(path.mEndpointId, path.mClusterId, path.mAttributeId, err);
ReturnErrorOnFailure(err);

if (!attributeState->Is<StatusIB>())
if (!attributeState->template Is<StatusIB>())
{
return CHIP_ERROR_INVALID_ARGUMENT;
}

status = attributeState->Get<StatusIB>();
status = attributeState->template Get<StatusIB>();
return CHIP_NO_ERROR;
}

CHIP_ERROR ClusterStateCache::GetStatus(const ConcreteEventPath & path, StatusIB & status) const
template <>
CHIP_ERROR ClusterStateCacheT<false>::GetStatus(const ConcreteAttributePath & path, StatusIB & status) const
{
return CHIP_ERROR_INVALID_ARGUMENT;
}

template <bool CanEnableDataCaching>
CHIP_ERROR ClusterStateCacheT<CanEnableDataCaching>::GetStatus(const ConcreteEventPath & path, StatusIB & status) const
{
auto statusIter = mEventStatusCache.find(path);
if (statusIter == mEventStatusCache.end())
Expand All @@ -446,7 +498,8 @@ CHIP_ERROR ClusterStateCache::GetStatus(const ConcreteEventPath & path, StatusIB
return CHIP_NO_ERROR;
}

void ClusterStateCache::GetSortedFilters(std::vector<std::pair<DataVersionFilter, size_t>> & aVector) const
template <bool CanEnableDataCaching>
void ClusterStateCacheT<CanEnableDataCaching>::GetSortedFilters(std::vector<std::pair<DataVersionFilter, size_t>> & aVector) const
{
for (auto const & endpointIter : mCache)
{
Expand All @@ -463,26 +516,33 @@ void ClusterStateCache::GetSortedFilters(std::vector<std::pair<DataVersionFilter

for (auto const & attributeIter : clusterIter.second.mAttributes)
{
if (attributeIter.second.Is<StatusIB>())
{
clusterSize += SizeOfStatusIB(attributeIter.second.Get<StatusIB>());
}
else if (attributeIter.second.Is<size_t>())
if constexpr (CanEnableDataCaching)
{
clusterSize += attributeIter.second.Get<size_t>();
if (attributeIter.second.template Is<StatusIB>())
{
clusterSize += SizeOfStatusIB(attributeIter.second.template Get<StatusIB>());
}
else if (attributeIter.second.template Is<uint32_t>())
{
clusterSize += attributeIter.second.template Get<uint32_t>();
}
else
{
VerifyOrDie(attributeIter.second.template Is<AttributeData>());
TLV::TLVReader bufReader;
bufReader.Init(attributeIter.second.template Get<AttributeData>().Get(),
attributeIter.second.template Get<AttributeData>().AllocatedSize());
ReturnOnFailure(bufReader.Next());
// Skip to the end of the element.
ReturnOnFailure(bufReader.Skip());

// Compute the amount of value data
clusterSize += bufReader.GetLengthRead();
}
}
else
{
VerifyOrDie(attributeIter.second.Is<AttributeData>());
TLV::TLVReader bufReader;
bufReader.Init(attributeIter.second.Get<AttributeData>().Get(),
attributeIter.second.Get<AttributeData>().AllocatedSize());
ReturnOnFailure(bufReader.Next());
// Skip to the end of the element.
ReturnOnFailure(bufReader.Skip());

// Compute the amount of value data
clusterSize += bufReader.GetLengthRead();
clusterSize += attributeIter.second;
}
}

Expand All @@ -505,9 +565,10 @@ void ClusterStateCache::GetSortedFilters(std::vector<std::pair<DataVersionFilter
});
}

CHIP_ERROR ClusterStateCache::OnUpdateDataVersionFilterList(DataVersionFilterIBs::Builder & aDataVersionFilterIBsBuilder,
const Span<AttributePathParams> & aAttributePaths,
bool & aEncodedDataVersionList)
template <bool CanEnableDataCaching>
CHIP_ERROR ClusterStateCacheT<CanEnableDataCaching>::OnUpdateDataVersionFilterList(
DataVersionFilterIBs::Builder & aDataVersionFilterIBsBuilder, const Span<AttributePathParams> & aAttributePaths,
bool & aEncodedDataVersionList)
{
CHIP_ERROR err = CHIP_NO_ERROR;
TLV::TLVWriter backup;
Expand Down Expand Up @@ -587,7 +648,8 @@ CHIP_ERROR ClusterStateCache::OnUpdateDataVersionFilterList(DataVersionFilterIBs
return err;
}

CHIP_ERROR ClusterStateCache::GetLastReportDataPath(ConcreteClusterPath & aPath)
template <bool CanEnableDataCaching>
CHIP_ERROR ClusterStateCacheT<CanEnableDataCaching>::GetLastReportDataPath(ConcreteClusterPath & aPath)
{
if (mLastReportDataPath.IsValidConcreteClusterPath())
{
Expand All @@ -596,5 +658,10 @@ CHIP_ERROR ClusterStateCache::GetLastReportDataPath(ConcreteClusterPath & aPath)
}
return CHIP_ERROR_INCORRECT_STATE;
}

// Ensure that our out-of-line template methods actually get compiled.
template class ClusterStateCacheT<true>;
template class ClusterStateCacheT<false>;

} // namespace app
} // namespace chip
Loading
Loading