Skip to content

Commit 2fcac2e

Browse files
bzbarsky-applebzbarsky-apple
authored
Make ClusterStateCache use less memory when not storing attribute values. (#33058)
* Make ClusterStateCache use less memory when not storing attribute values. ClusterStateCache has a "no storing values" mode, but in that mode it still uses 24 bytes per attribute to store just the data size. For a typical device with a few hundred attributes, this adds up to multiple KB of RAM; across many devices it really starts to add up. Introduce a compile-time switch that lets us optimize the storage when we know we're not storing the data. This still keeps the old "enable storing data at compile time but turn it off at runtime" API, both for API compat and because it might reduce codesize for some consumers who care about that more than RAM. * Address review comment. * Actually fix the review comment.
1 parent af57567 commit 2fcac2e

File tree

2 files changed

+157
-75
lines changed

2 files changed

+157
-75
lines changed

‎src/app/ClusterStateCache.cpp

+129-62
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,14 @@ namespace app {
2727
namespace {
2828

2929
// Determine how much space a StatusIB takes up on the wire.
30-
size_t SizeOfStatusIB(const StatusIB & aStatus)
30+
uint32_t SizeOfStatusIB(const StatusIB & aStatus)
3131
{
3232
// 1 byte: anonymous tag control byte for struct.
3333
// 1 byte: control byte for uint8 value.
3434
// 1 byte: context-specific tag for uint8 value.
3535
// 1 byte: the uint8 value.
3636
// 1 byte: end of container.
37-
size_t size = 5;
37+
uint32_t size = 5;
3838

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

5050
} // anonymous namespace
5151

52-
CHIP_ERROR ClusterStateCache::GetElementTLVSize(TLV::TLVReader * apData, size_t & aSize)
52+
template <bool CanEnableDataCaching>
53+
CHIP_ERROR ClusterStateCacheT<CanEnableDataCaching>::GetElementTLVSize(TLV::TLVReader * apData, uint32_t & aSize)
5354
{
5455
Platform::ScopedMemoryBufferWithSize<uint8_t> backingBuffer;
5556
TLV::TLVReader reader;
@@ -64,8 +65,9 @@ CHIP_ERROR ClusterStateCache::GetElementTLVSize(TLV::TLVReader * apData, size_t
6465
return CHIP_NO_ERROR;
6566
}
6667

67-
CHIP_ERROR ClusterStateCache::UpdateCache(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData,
68-
const StatusIB & aStatus)
68+
template <bool CanEnableDataCaching>
69+
CHIP_ERROR ClusterStateCacheT<CanEnableDataCaching>::UpdateCache(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData,
70+
const StatusIB & aStatus)
6971
{
7072
AttributeState state;
7173
bool endpointIsNew = false;
@@ -82,24 +84,32 @@ CHIP_ERROR ClusterStateCache::UpdateCache(const ConcreteDataAttributePath & aPat
8284

8385
if (apData)
8486
{
85-
size_t elementSize = 0;
87+
uint32_t elementSize = 0;
8688
ReturnErrorOnFailure(GetElementTLVSize(apData, elementSize));
8789

88-
if (mCacheData)
90+
if constexpr (CanEnableDataCaching)
8991
{
90-
Platform::ScopedMemoryBufferWithSize<uint8_t> backingBuffer;
91-
backingBuffer.Calloc(elementSize);
92-
VerifyOrReturnError(backingBuffer.Get() != nullptr, CHIP_ERROR_NO_MEMORY);
93-
TLV::ScopedBufferTLVWriter writer(std::move(backingBuffer), elementSize);
94-
ReturnErrorOnFailure(writer.CopyElement(TLV::AnonymousTag(), *apData));
95-
ReturnErrorOnFailure(writer.Finalize(backingBuffer));
96-
97-
state.Set<AttributeData>(std::move(backingBuffer));
92+
if (mCacheData)
93+
{
94+
Platform::ScopedMemoryBufferWithSize<uint8_t> backingBuffer;
95+
backingBuffer.Calloc(elementSize);
96+
VerifyOrReturnError(backingBuffer.Get() != nullptr, CHIP_ERROR_NO_MEMORY);
97+
TLV::ScopedBufferTLVWriter writer(std::move(backingBuffer), elementSize);
98+
ReturnErrorOnFailure(writer.CopyElement(TLV::AnonymousTag(), *apData));
99+
ReturnErrorOnFailure(writer.Finalize(backingBuffer));
100+
101+
state.template Set<AttributeData>(std::move(backingBuffer));
102+
}
103+
else
104+
{
105+
state.template Set<uint32_t>(elementSize);
106+
}
98107
}
99108
else
100109
{
101-
state.Set<size_t>(elementSize);
110+
state = elementSize;
102111
}
112+
103113
//
104114
// Clear out the committed data version and only set it again once we have received all data for this cluster.
105115
// Otherwise, we may have incomplete data that looks like it's complete since it has a valid data version.
@@ -132,13 +142,20 @@ CHIP_ERROR ClusterStateCache::UpdateCache(const ConcreteDataAttributePath & aPat
132142
}
133143
else
134144
{
135-
if (mCacheData)
145+
if constexpr (CanEnableDataCaching)
136146
{
137-
state.Set<StatusIB>(aStatus);
147+
if (mCacheData)
148+
{
149+
state.template Set<StatusIB>(aStatus);
150+
}
151+
else
152+
{
153+
state.template Set<uint32_t>(SizeOfStatusIB(aStatus));
154+
}
138155
}
139156
else
140157
{
141-
state.Set<size_t>(SizeOfStatusIB(aStatus));
158+
state = SizeOfStatusIB(aStatus);
142159
}
143160
}
144161

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

164-
CHIP_ERROR ClusterStateCache::UpdateEventCache(const EventHeader & aEventHeader, TLV::TLVReader * apData, const StatusIB * apStatus)
181+
template <bool CanEnableDataCaching>
182+
CHIP_ERROR ClusterStateCacheT<CanEnableDataCaching>::UpdateEventCache(const EventHeader & aEventHeader, TLV::TLVReader * apData,
183+
const StatusIB * apStatus)
165184
{
166185
if (apData)
167186
{
@@ -208,15 +227,17 @@ CHIP_ERROR ClusterStateCache::UpdateEventCache(const EventHeader & aEventHeader,
208227
return CHIP_NO_ERROR;
209228
}
210229

211-
void ClusterStateCache::OnReportBegin()
230+
template <bool CanEnableDataCaching>
231+
void ClusterStateCacheT<CanEnableDataCaching>::OnReportBegin()
212232
{
213233
mLastReportDataPath = ConcreteClusterPath(kInvalidEndpointId, kInvalidClusterId);
214234
mChangedAttributeSet.clear();
215235
mAddedEndpoints.clear();
216236
mCallback.OnReportBegin();
217237
}
218238

219-
void ClusterStateCache::CommitPendingDataVersion()
239+
template <bool CanEnableDataCaching>
240+
void ClusterStateCacheT<CanEnableDataCaching>::CommitPendingDataVersion()
220241
{
221242
if (!mLastReportDataPath.IsValidConcreteClusterPath())
222243
{
@@ -231,7 +252,8 @@ void ClusterStateCache::CommitPendingDataVersion()
231252
}
232253
}
233254

234-
void ClusterStateCache::OnReportEnd()
255+
template <bool CanEnableDataCaching>
256+
void ClusterStateCacheT<CanEnableDataCaching>::OnReportEnd()
235257
{
236258
CommitPendingDataVersion();
237259
mLastReportDataPath = ConcreteClusterPath(kInvalidEndpointId, kInvalidClusterId);
@@ -260,26 +282,35 @@ void ClusterStateCache::OnReportEnd()
260282
mCallback.OnReportEnd();
261283
}
262284

263-
CHIP_ERROR ClusterStateCache::Get(const ConcreteAttributePath & path, TLV::TLVReader & reader) const
285+
template <>
286+
CHIP_ERROR ClusterStateCacheT<true>::Get(const ConcreteAttributePath & path, TLV::TLVReader & reader) const
264287
{
265288
CHIP_ERROR err;
266289
auto attributeState = GetAttributeState(path.mEndpointId, path.mClusterId, path.mAttributeId, err);
267290
ReturnErrorOnFailure(err);
268-
if (attributeState->Is<StatusIB>())
291+
292+
if (attributeState->template Is<StatusIB>())
269293
{
270294
return CHIP_ERROR_IM_STATUS_CODE_RECEIVED;
271295
}
272296

273-
if (!attributeState->Is<AttributeData>())
297+
if (!attributeState->template Is<AttributeData>())
274298
{
275299
return CHIP_ERROR_KEY_NOT_FOUND;
276300
}
277301

278-
reader.Init(attributeState->Get<AttributeData>().Get(), attributeState->Get<AttributeData>().AllocatedSize());
302+
reader.Init(attributeState->template Get<AttributeData>().Get(), attributeState->template Get<AttributeData>().AllocatedSize());
279303
return reader.Next();
280304
}
281305

282-
CHIP_ERROR ClusterStateCache::Get(EventNumber eventNumber, TLV::TLVReader & reader) const
306+
template <>
307+
CHIP_ERROR ClusterStateCacheT<false>::Get(const ConcreteAttributePath & path, TLV::TLVReader & reader) const
308+
{
309+
return CHIP_ERROR_KEY_NOT_FOUND;
310+
}
311+
312+
template <bool CanEnableDataCaching>
313+
CHIP_ERROR ClusterStateCacheT<CanEnableDataCaching>::Get(EventNumber eventNumber, TLV::TLVReader & reader) const
283314
{
284315
CHIP_ERROR err;
285316

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

298-
const ClusterStateCache::EndpointState * ClusterStateCache::GetEndpointState(EndpointId endpointId, CHIP_ERROR & err) const
329+
template <bool CanEnableDataCaching>
330+
const typename ClusterStateCacheT<CanEnableDataCaching>::EndpointState *
331+
ClusterStateCacheT<CanEnableDataCaching>::GetEndpointState(EndpointId endpointId, CHIP_ERROR & err) const
299332
{
300333
auto endpointIter = mCache.find(endpointId);
301334
if (endpointIter == mCache.end())
@@ -308,8 +341,9 @@ const ClusterStateCache::EndpointState * ClusterStateCache::GetEndpointState(End
308341
return &endpointIter->second;
309342
}
310343

311-
const ClusterStateCache::ClusterState * ClusterStateCache::GetClusterState(EndpointId endpointId, ClusterId clusterId,
312-
CHIP_ERROR & err) const
344+
template <bool CanEnableDataCaching>
345+
const typename ClusterStateCacheT<CanEnableDataCaching>::ClusterState *
346+
ClusterStateCacheT<CanEnableDataCaching>::GetClusterState(EndpointId endpointId, ClusterId clusterId, CHIP_ERROR & err) const
313347
{
314348
auto endpointState = GetEndpointState(endpointId, err);
315349
if (err != CHIP_NO_ERROR)
@@ -328,8 +362,10 @@ const ClusterStateCache::ClusterState * ClusterStateCache::GetClusterState(Endpo
328362
return &clusterState->second;
329363
}
330364

331-
const ClusterStateCache::AttributeState * ClusterStateCache::GetAttributeState(EndpointId endpointId, ClusterId clusterId,
332-
AttributeId attributeId, CHIP_ERROR & err) const
365+
template <bool CanEnableDataCaching>
366+
const typename ClusterStateCacheT<CanEnableDataCaching>::AttributeState *
367+
ClusterStateCacheT<CanEnableDataCaching>::GetAttributeState(EndpointId endpointId, ClusterId clusterId, AttributeId attributeId,
368+
CHIP_ERROR & err) const
333369
{
334370
auto clusterState = GetClusterState(endpointId, clusterId, err);
335371
if (err != CHIP_NO_ERROR)
@@ -348,7 +384,9 @@ const ClusterStateCache::AttributeState * ClusterStateCache::GetAttributeState(E
348384
return &attributeState->second;
349385
}
350386

351-
const ClusterStateCache::EventData * ClusterStateCache::GetEventData(EventNumber eventNumber, CHIP_ERROR & err) const
387+
template <bool CanEnableDataCaching>
388+
const typename ClusterStateCacheT<CanEnableDataCaching>::EventData *
389+
ClusterStateCacheT<CanEnableDataCaching>::GetEventData(EventNumber eventNumber, CHIP_ERROR & err) const
352390
{
353391
EventData compareKey;
354392

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

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

397-
CHIP_ERROR ClusterStateCache::GetVersion(const ConcreteClusterPath & aPath, Optional<DataVersion> & aVersion) const
437+
template <bool CanEnableDataCaching>
438+
CHIP_ERROR ClusterStateCacheT<CanEnableDataCaching>::GetVersion(const ConcreteClusterPath & aPath,
439+
Optional<DataVersion> & aVersion) const
398440
{
399441
VerifyOrReturnError(aPath.IsValidConcreteClusterPath(), CHIP_ERROR_INVALID_ARGUMENT);
400442
CHIP_ERROR err;
@@ -404,7 +446,9 @@ CHIP_ERROR ClusterStateCache::GetVersion(const ConcreteClusterPath & aPath, Opti
404446
return CHIP_NO_ERROR;
405447
}
406448

407-
void ClusterStateCache::OnEventData(const EventHeader & aEventHeader, TLV::TLVReader * apData, const StatusIB * apStatus)
449+
template <bool CanEnableDataCaching>
450+
void ClusterStateCacheT<CanEnableDataCaching>::OnEventData(const EventHeader & aEventHeader, TLV::TLVReader * apData,
451+
const StatusIB * apStatus)
408452
{
409453
VerifyOrDie(apData != nullptr || apStatus != nullptr);
410454

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

421-
CHIP_ERROR ClusterStateCache::GetStatus(const ConcreteAttributePath & path, StatusIB & status) const
465+
template <>
466+
CHIP_ERROR ClusterStateCacheT<true>::GetStatus(const ConcreteAttributePath & path, StatusIB & status) const
422467
{
423468
CHIP_ERROR err;
424469

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

428-
if (!attributeState->Is<StatusIB>())
473+
if (!attributeState->template Is<StatusIB>())
429474
{
430475
return CHIP_ERROR_INVALID_ARGUMENT;
431476
}
432477

433-
status = attributeState->Get<StatusIB>();
478+
status = attributeState->template Get<StatusIB>();
434479
return CHIP_NO_ERROR;
435480
}
436481

437-
CHIP_ERROR ClusterStateCache::GetStatus(const ConcreteEventPath & path, StatusIB & status) const
482+
template <>
483+
CHIP_ERROR ClusterStateCacheT<false>::GetStatus(const ConcreteAttributePath & path, StatusIB & status) const
484+
{
485+
return CHIP_ERROR_INVALID_ARGUMENT;
486+
}
487+
488+
template <bool CanEnableDataCaching>
489+
CHIP_ERROR ClusterStateCacheT<CanEnableDataCaching>::GetStatus(const ConcreteEventPath & path, StatusIB & status) const
438490
{
439491
auto statusIter = mEventStatusCache.find(path);
440492
if (statusIter == mEventStatusCache.end())
@@ -446,7 +498,8 @@ CHIP_ERROR ClusterStateCache::GetStatus(const ConcreteEventPath & path, StatusIB
446498
return CHIP_NO_ERROR;
447499
}
448500

449-
void ClusterStateCache::GetSortedFilters(std::vector<std::pair<DataVersionFilter, size_t>> & aVector) const
501+
template <bool CanEnableDataCaching>
502+
void ClusterStateCacheT<CanEnableDataCaching>::GetSortedFilters(std::vector<std::pair<DataVersionFilter, size_t>> & aVector) const
450503
{
451504
for (auto const & endpointIter : mCache)
452505
{
@@ -463,26 +516,33 @@ void ClusterStateCache::GetSortedFilters(std::vector<std::pair<DataVersionFilter
463516

464517
for (auto const & attributeIter : clusterIter.second.mAttributes)
465518
{
466-
if (attributeIter.second.Is<StatusIB>())
467-
{
468-
clusterSize += SizeOfStatusIB(attributeIter.second.Get<StatusIB>());
469-
}
470-
else if (attributeIter.second.Is<size_t>())
519+
if constexpr (CanEnableDataCaching)
471520
{
472-
clusterSize += attributeIter.second.Get<size_t>();
521+
if (attributeIter.second.template Is<StatusIB>())
522+
{
523+
clusterSize += SizeOfStatusIB(attributeIter.second.template Get<StatusIB>());
524+
}
525+
else if (attributeIter.second.template Is<uint32_t>())
526+
{
527+
clusterSize += attributeIter.second.template Get<uint32_t>();
528+
}
529+
else
530+
{
531+
VerifyOrDie(attributeIter.second.template Is<AttributeData>());
532+
TLV::TLVReader bufReader;
533+
bufReader.Init(attributeIter.second.template Get<AttributeData>().Get(),
534+
attributeIter.second.template Get<AttributeData>().AllocatedSize());
535+
ReturnOnFailure(bufReader.Next());
536+
// Skip to the end of the element.
537+
ReturnOnFailure(bufReader.Skip());
538+
539+
// Compute the amount of value data
540+
clusterSize += bufReader.GetLengthRead();
541+
}
473542
}
474543
else
475544
{
476-
VerifyOrDie(attributeIter.second.Is<AttributeData>());
477-
TLV::TLVReader bufReader;
478-
bufReader.Init(attributeIter.second.Get<AttributeData>().Get(),
479-
attributeIter.second.Get<AttributeData>().AllocatedSize());
480-
ReturnOnFailure(bufReader.Next());
481-
// Skip to the end of the element.
482-
ReturnOnFailure(bufReader.Skip());
483-
484-
// Compute the amount of value data
485-
clusterSize += bufReader.GetLengthRead();
545+
clusterSize += attributeIter.second;
486546
}
487547
}
488548

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

508-
CHIP_ERROR ClusterStateCache::OnUpdateDataVersionFilterList(DataVersionFilterIBs::Builder & aDataVersionFilterIBsBuilder,
509-
const Span<AttributePathParams> & aAttributePaths,
510-
bool & aEncodedDataVersionList)
568+
template <bool CanEnableDataCaching>
569+
CHIP_ERROR ClusterStateCacheT<CanEnableDataCaching>::OnUpdateDataVersionFilterList(
570+
DataVersionFilterIBs::Builder & aDataVersionFilterIBsBuilder, const Span<AttributePathParams> & aAttributePaths,
571+
bool & aEncodedDataVersionList)
511572
{
512573
CHIP_ERROR err = CHIP_NO_ERROR;
513574
TLV::TLVWriter backup;
@@ -587,7 +648,8 @@ CHIP_ERROR ClusterStateCache::OnUpdateDataVersionFilterList(DataVersionFilterIBs
587648
return err;
588649
}
589650

590-
CHIP_ERROR ClusterStateCache::GetLastReportDataPath(ConcreteClusterPath & aPath)
651+
template <bool CanEnableDataCaching>
652+
CHIP_ERROR ClusterStateCacheT<CanEnableDataCaching>::GetLastReportDataPath(ConcreteClusterPath & aPath)
591653
{
592654
if (mLastReportDataPath.IsValidConcreteClusterPath())
593655
{
@@ -596,5 +658,10 @@ CHIP_ERROR ClusterStateCache::GetLastReportDataPath(ConcreteClusterPath & aPath)
596658
}
597659
return CHIP_ERROR_INCORRECT_STATE;
598660
}
661+
662+
// Ensure that our out-of-line template methods actually get compiled.
663+
template class ClusterStateCacheT<true>;
664+
template class ClusterStateCacheT<false>;
665+
599666
} // namespace app
600667
} // namespace chip

0 commit comments

Comments
 (0)