Skip to content

Commit 3ddf53d

Browse files
jtung-applebzbarsky-applewoody-apple
authored
[Darwin] MTRDeviceControllerStorageDelegate should support bulk store/fetch (#32858)
* [Darwin] MTRDeviceControllerStorageDelegate should support bulk store/fetch * Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * MTRDeviceControllerStorageDelegate header documentation clarification * Update src/darwin/Framework/CHIP/MTRDevice.mm Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/darwin/Framework/CHIP/MTRDeviceController.mm Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Address review comments * Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/darwin/Framework/CHIP/MTRDeviceControllerStorageDelegate.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Move processing out of storage delegate queue dispatch_sync --------- Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> Co-authored-by: Justin Wood <woody@apple.com>
1 parent 2dd3bfd commit 3ddf53d

11 files changed

+530
-52
lines changed

‎src/darwin/Framework/CHIP/MTRDevice.mm

+22
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,20 @@ - (id)copyWithZone:(NSZone *)zone
236236
return [[MTRDeviceClusterData alloc] initWithDataVersion:_dataVersion attributes:_attributes];
237237
}
238238

239+
- (BOOL)isEqualToClusterData:(MTRDeviceClusterData *)otherClusterData
240+
{
241+
return [_dataVersion isEqual:otherClusterData.dataVersion] && [_attributes isEqual:otherClusterData.attributes];
242+
}
243+
244+
- (BOOL)isEqual:(id)object
245+
{
246+
if ([object class] != [self class]) {
247+
return NO;
248+
}
249+
250+
return [self isEqualToClusterData:object];
251+
}
252+
239253
@end
240254

241255
@interface MTRDevice ()
@@ -2250,6 +2264,14 @@ - (void)setAttributeValues:(NSArray<NSDictionary *> *)attributeValues reportChan
22502264
[self _setAttributeValues:attributeValues reportChanges:reportChanges];
22512265
}
22522266

2267+
#ifdef DEBUG
2268+
- (NSUInteger)unitTestAttributeCount
2269+
{
2270+
std::lock_guard lock(_lock);
2271+
return _readCache.count;
2272+
}
2273+
#endif
2274+
22532275
- (void)setClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *)clusterData
22542276
{
22552277
MTR_LOG_INFO("%@ setClusterData count: %lu", self, static_cast<unsigned long>(clusterData.count));

‎src/darwin/Framework/CHIP/MTRDeviceController.mm

+54-12
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,20 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams
576576
return NO;
577577
}
578578

579+
if (_controllerDataStore) {
580+
// If the storage delegate supports the bulk read API, then a dictionary of nodeID => cluster data dictionary would be passed to the handler. Otherwise this would be a no-op, and stored attributes for MTRDevice objects will be loaded lazily in -deviceForNodeID:.
581+
[_controllerDataStore fetchAttributeDataForAllDevices:^(NSDictionary<NSNumber *, NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *> * _Nonnull clusterDataByNode) {
582+
MTR_LOG_INFO("Loaded attribute values for %lu nodes from storage for controller uuid %@", static_cast<unsigned long>(clusterDataByNode.count), self->_uniqueIdentifier);
583+
584+
std::lock_guard lock(self->_deviceMapLock);
585+
for (NSNumber * nodeID in clusterDataByNode) {
586+
NSDictionary * clusterData = clusterDataByNode[nodeID];
587+
MTRDevice * device = [self _setupDeviceForNodeID:nodeID prefetchedClusterData:clusterData];
588+
MTR_LOG_INFO("Loaded %lu cluster data from storage for %@", static_cast<unsigned long>(clusterData.count), device);
589+
}
590+
}];
591+
}
592+
579593
return YES;
580594
}
581595

@@ -919,20 +933,25 @@ - (MTRBaseDevice *)baseDeviceForNodeID:(NSNumber *)nodeID
919933
return [[MTRBaseDevice alloc] initWithNodeID:nodeID controller:self];
920934
}
921935

922-
- (MTRDevice *)deviceForNodeID:(NSNumber *)nodeID
936+
// If prefetchedClusterData is not provided, load attributes individually from controller data store
937+
- (MTRDevice *)_setupDeviceForNodeID:(NSNumber *)nodeID prefetchedClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *)prefetchedClusterData
923938
{
924-
std::lock_guard lock(_deviceMapLock);
925-
MTRDevice * deviceToReturn = _nodeIDToDeviceMap[nodeID];
926-
if (!deviceToReturn) {
927-
deviceToReturn = [[MTRDevice alloc] initWithNodeID:nodeID controller:self];
928-
// If we're not running, don't add the device to our map. That would
929-
// create a cycle that nothing would break. Just return the device,
930-
// which will be in exactly the state it would be in if it were created
931-
// while we were running and then we got shut down.
932-
if ([self isRunning]) {
933-
_nodeIDToDeviceMap[nodeID] = deviceToReturn;
934-
}
939+
os_unfair_lock_assert_owner(&_deviceMapLock);
940+
941+
MTRDevice * deviceToReturn = [[MTRDevice alloc] initWithNodeID:nodeID controller:self];
942+
// If we're not running, don't add the device to our map. That would
943+
// create a cycle that nothing would break. Just return the device,
944+
// which will be in exactly the state it would be in if it were created
945+
// while we were running and then we got shut down.
946+
if ([self isRunning]) {
947+
_nodeIDToDeviceMap[nodeID] = deviceToReturn;
948+
}
935949

950+
if (prefetchedClusterData) {
951+
if (prefetchedClusterData.count) {
952+
[deviceToReturn setClusterData:prefetchedClusterData];
953+
}
954+
} else {
936955
#if !MTRDEVICE_ATTRIBUTE_CACHE_STORE_ATTRIBUTES_BY_CLUSTER
937956
// Load persisted attributes if they exist.
938957
NSArray * attributesFromCache = [_controllerDataStore getStoredAttributesForNodeID:nodeID];
@@ -952,6 +971,17 @@ - (MTRDevice *)deviceForNodeID:(NSNumber *)nodeID
952971
return deviceToReturn;
953972
}
954973

974+
- (MTRDevice *)deviceForNodeID:(NSNumber *)nodeID
975+
{
976+
std::lock_guard lock(_deviceMapLock);
977+
MTRDevice * deviceToReturn = _nodeIDToDeviceMap[nodeID];
978+
if (!deviceToReturn) {
979+
deviceToReturn = [self _setupDeviceForNodeID:nodeID prefetchedClusterData:nil];
980+
}
981+
982+
return deviceToReturn;
983+
}
984+
955985
- (void)removeDevice:(MTRDevice *)device
956986
{
957987
std::lock_guard lock(_deviceMapLock);
@@ -965,6 +995,18 @@ - (void)removeDevice:(MTRDevice *)device
965995
}
966996
}
967997

998+
#ifdef DEBUG
999+
- (NSDictionary<NSNumber *, NSNumber *> *)unitTestGetDeviceAttributeCounts
1000+
{
1001+
std::lock_guard lock(_deviceMapLock);
1002+
NSMutableDictionary<NSNumber *, NSNumber *> * deviceAttributeCounts = [NSMutableDictionary dictionary];
1003+
for (NSNumber * nodeID in _nodeIDToDeviceMap) {
1004+
deviceAttributeCounts[nodeID] = @([_nodeIDToDeviceMap[nodeID] unitTestAttributeCount]);
1005+
}
1006+
return deviceAttributeCounts;
1007+
}
1008+
#endif
1009+
9681010
- (void)setDeviceControllerDelegate:(id<MTRDeviceControllerDelegate>)delegate queue:(dispatch_queue_t)queue
9691011
{
9701012
[self

‎src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h

+9
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,15 @@ NS_ASSUME_NONNULL_BEGIN
4444
storageDelegate:(id<MTRDeviceControllerStorageDelegate>)storageDelegate
4545
storageDelegateQueue:(dispatch_queue_t)storageDelegateQueue;
4646

47+
// clusterDataByNode a dictionary: nodeID => cluster data dictionary
48+
typedef void (^MTRDeviceControllerDataStoreClusterDataHandler)(NSDictionary<NSNumber *, NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *> * clusterDataByNode);
49+
50+
/**
51+
* Asks the data store to load cluster data for nodes in bulk. If the storageDelegate supports it, the handler will be called synchronously.
52+
* If the storageDelegate does not support it, the handler will not be called at all.
53+
*/
54+
- (void)fetchAttributeDataForAllDevices:(MTRDeviceControllerDataStoreClusterDataHandler)clusterDataHandler;
55+
4756
/**
4857
* Resumption info APIs.
4958
*/

‎src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm

+147-22
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,20 @@ - (nullable instancetype)initWithController:(MTRDeviceController *)controller
151151
return self;
152152
}
153153

154+
- (void)fetchAttributeDataForAllDevices:(MTRDeviceControllerDataStoreClusterDataHandler)clusterDataHandler
155+
{
156+
__block NSDictionary<NSString *, id> * dataStoreSecureLocalValues = nil;
157+
dispatch_sync(_storageDelegateQueue, ^{
158+
if ([self->_storageDelegate respondsToSelector:@selector(valuesForController:securityLevel:sharingType:)]) {
159+
dataStoreSecureLocalValues = [self->_storageDelegate valuesForController:self->_controller securityLevel:MTRStorageSecurityLevelSecure sharingType:MTRStorageSharingTypeNotShared];
160+
}
161+
});
162+
163+
if (dataStoreSecureLocalValues.count) {
164+
clusterDataHandler([self _getClusterDataFromSecureLocalValues:dataStoreSecureLocalValues]);
165+
}
166+
}
167+
154168
- (nullable MTRCASESessionResumptionInfo *)findResumptionInfoByNodeID:(NSNumber *)nodeID
155169
{
156170
return [self _findResumptionInfoWithKey:ResumptionByNodeIDKey(nodeID)];
@@ -337,6 +351,14 @@ - (BOOL)_storeAttributeCacheValue:(id)value forKey:(NSString *)key
337351
sharingType:MTRStorageSharingTypeNotShared];
338352
}
339353

354+
- (BOOL)_bulkStoreAttributeCacheValues:(NSDictionary<NSString *, id<NSSecureCoding>> *)values
355+
{
356+
return [_storageDelegate controller:_controller
357+
storeValues:values
358+
securityLevel:MTRStorageSecurityLevelSecure
359+
sharingType:MTRStorageSharingTypeNotShared];
360+
}
361+
340362
- (BOOL)_removeAttributeCacheValueForKey:(NSString *)key
341363
{
342364
return [_storageDelegate controller:_controller
@@ -968,6 +990,76 @@ - (void)clearAllStoredAttributes
968990
return clusterDataToReturn;
969991
}
970992

993+
// Utility for constructing dictionary of nodeID to cluster data from dictionary of storage keys
994+
- (nullable NSDictionary<NSNumber *, NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *> *)_getClusterDataFromSecureLocalValues:(NSDictionary<NSString *, id> *)secureLocalValues
995+
{
996+
NSMutableDictionary<NSNumber *, NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *> * clusterDataByNodeToReturn = nil;
997+
998+
if (![secureLocalValues isKindOfClass:[NSDictionary class]]) {
999+
return nil;
1000+
}
1001+
1002+
// Fetch node index
1003+
NSArray<NSNumber *> * nodeIndex = secureLocalValues[sAttributeCacheNodeIndexKey];
1004+
1005+
if (![nodeIndex isKindOfClass:[NSArray class]]) {
1006+
return nil;
1007+
}
1008+
1009+
for (NSNumber * nodeID in nodeIndex) {
1010+
if (![nodeID isKindOfClass:[NSNumber class]]) {
1011+
continue;
1012+
}
1013+
1014+
NSMutableDictionary<MTRClusterPath *, MTRDeviceClusterData *> * clusterDataForNode = nil;
1015+
NSArray<NSNumber *> * endpointIndex = secureLocalValues[[self _endpointIndexKeyForNodeID:nodeID]];
1016+
1017+
if (![endpointIndex isKindOfClass:[NSArray class]]) {
1018+
continue;
1019+
}
1020+
1021+
for (NSNumber * endpointID in endpointIndex) {
1022+
if (![endpointID isKindOfClass:[NSNumber class]]) {
1023+
continue;
1024+
}
1025+
1026+
NSArray<NSNumber *> * clusterIndex = secureLocalValues[[self _clusterIndexKeyForNodeID:nodeID endpointID:endpointID]];
1027+
1028+
if (![clusterIndex isKindOfClass:[NSArray class]]) {
1029+
continue;
1030+
}
1031+
1032+
for (NSNumber * clusterID in clusterIndex) {
1033+
if (![clusterID isKindOfClass:[NSNumber class]]) {
1034+
continue;
1035+
}
1036+
1037+
MTRDeviceClusterData * clusterData = secureLocalValues[[self _clusterDataKeyForNodeID:nodeID endpointID:endpointID clusterID:clusterID]];
1038+
if (!clusterData) {
1039+
continue;
1040+
}
1041+
if (![clusterData isKindOfClass:[MTRDeviceClusterData class]]) {
1042+
continue;
1043+
}
1044+
MTRClusterPath * clusterPath = [MTRClusterPath clusterPathWithEndpointID:endpointID clusterID:clusterID];
1045+
if (!clusterDataForNode) {
1046+
clusterDataForNode = [NSMutableDictionary dictionary];
1047+
}
1048+
clusterDataForNode[clusterPath] = clusterData;
1049+
}
1050+
}
1051+
1052+
if (clusterDataForNode.count) {
1053+
if (!clusterDataByNodeToReturn) {
1054+
clusterDataByNodeToReturn = [NSMutableDictionary dictionary];
1055+
}
1056+
clusterDataByNodeToReturn[nodeID] = clusterDataForNode;
1057+
}
1058+
}
1059+
1060+
return clusterDataByNodeToReturn;
1061+
}
1062+
9711063
- (void)storeClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *)clusterData forNodeID:(NSNumber *)nodeID
9721064
{
9731065
if (!nodeID) {
@@ -983,6 +1075,11 @@ - (void)storeClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *>
9831075
dispatch_async(_storageDelegateQueue, ^{
9841076
NSUInteger storeFailures = 0;
9851077

1078+
NSMutableDictionary<NSString *, id<NSSecureCoding>> * bulkValuesToStore = nil;
1079+
if ([self->_storageDelegate respondsToSelector:@selector(controller:storeValues:securityLevel:sharingType:)]) {
1080+
bulkValuesToStore = [NSMutableDictionary dictionary];
1081+
}
1082+
9861083
// A map of endpoint => list of clusters modified for that endpoint so cluster indexes can be updated later
9871084
NSMutableDictionary<NSNumber *, NSMutableSet<NSNumber *> *> * clustersModified = [NSMutableDictionary dictionary];
9881085

@@ -993,11 +1090,15 @@ - (void)storeClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *>
9931090
MTR_LOG_INFO("Attempt to store clusterData @ node 0x%016llX endpoint %u cluster 0x%08lX", nodeID.unsignedLongLongValue, path.endpoint.unsignedShortValue, path.cluster.unsignedLongValue);
9941091
#endif
9951092

996-
// Store cluster data
997-
BOOL storeFailed = ![self _storeClusterData:data forNodeID:nodeID endpointID:path.endpoint clusterID:path.cluster];
998-
if (storeFailed) {
999-
storeFailures++;
1000-
MTR_LOG_INFO("Store failed for clusterDAta @ node 0x%016llX endpoint %u cluster 0x%08lX", nodeID.unsignedLongLongValue, path.endpoint.unsignedShortValue, path.cluster.unsignedLongValue);
1093+
if (bulkValuesToStore) {
1094+
bulkValuesToStore[[self _clusterDataKeyForNodeID:nodeID endpointID:path.endpoint clusterID:path.cluster]] = data;
1095+
} else {
1096+
// Store cluster data
1097+
BOOL storeFailed = ![self _storeClusterData:data forNodeID:nodeID endpointID:path.endpoint clusterID:path.cluster];
1098+
if (storeFailed) {
1099+
storeFailures++;
1100+
MTR_LOG_INFO("Store failed for clusterData @ node 0x%016llX endpoint %u cluster 0x%08lX", nodeID.unsignedLongLongValue, path.endpoint.unsignedShortValue, path.cluster.unsignedLongValue);
1101+
}
10011102
}
10021103

10031104
// Note the cluster as modified for the endpoint
@@ -1046,36 +1147,60 @@ - (void)storeClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *>
10461147
}
10471148

10481149
if (clusterIndexModified) {
1049-
BOOL storeFailed = ![self _storeClusterIndex:clusterIndexToStore forNodeID:nodeID endpointID:endpointID];
1050-
if (storeFailed) {
1051-
storeFailures++;
1052-
MTR_LOG_INFO("Store failed for clusterIndex @ node 0x%016llX endpoint %u", nodeID.unsignedLongLongValue, endpointID.unsignedShortValue);
1053-
continue;
1150+
if (bulkValuesToStore) {
1151+
bulkValuesToStore[[self _clusterIndexKeyForNodeID:nodeID endpointID:endpointID]] = clusterIndexToStore;
1152+
} else {
1153+
BOOL storeFailed = ![self _storeClusterIndex:clusterIndexToStore forNodeID:nodeID endpointID:endpointID];
1154+
if (storeFailed) {
1155+
storeFailures++;
1156+
MTR_LOG_INFO("Store failed for clusterIndex @ node 0x%016llX endpoint %u", nodeID.unsignedLongLongValue, endpointID.unsignedShortValue);
1157+
continue;
1158+
}
10541159
}
10551160
}
10561161
}
10571162

10581163
// Update endpoint index as needed
10591164
if (endpointIndexModified) {
1060-
BOOL storeFailed = ![self _storeEndpointIndex:endpointIndexToStore forNodeID:nodeID];
1061-
if (storeFailed) {
1062-
storeFailures++;
1063-
MTR_LOG_INFO("Store failed for endpointIndex @ node 0x%016llX", nodeID.unsignedLongLongValue);
1165+
if (bulkValuesToStore) {
1166+
bulkValuesToStore[[self _endpointIndexKeyForNodeID:nodeID]] = endpointIndexToStore;
1167+
} else {
1168+
BOOL storeFailed = ![self _storeEndpointIndex:endpointIndexToStore forNodeID:nodeID];
1169+
if (storeFailed) {
1170+
storeFailures++;
1171+
MTR_LOG_INFO("Store failed for endpointIndex @ node 0x%016llX", nodeID.unsignedLongLongValue);
1172+
}
10641173
}
10651174
}
10661175

1067-
// Ensure node index exists
1176+
// Check if node index needs updating / creation
10681177
NSArray<NSNumber *> * nodeIndex = [self _fetchNodeIndex];
1069-
BOOL storeFailed = NO;
1178+
NSArray<NSNumber *> * nodeIndexToStore = nil;
10701179
if (!nodeIndex) {
1071-
nodeIndex = [NSArray arrayWithObject:nodeID];
1072-
storeFailed = ![self _storeNodeIndex:nodeIndex];
1180+
// Ensure node index exists
1181+
nodeIndexToStore = [NSArray arrayWithObject:nodeID];
10731182
} else if (![nodeIndex containsObject:nodeID]) {
1074-
storeFailed = ![self _storeNodeIndex:[nodeIndex arrayByAddingObject:nodeID]];
1183+
nodeIndexToStore = [nodeIndex arrayByAddingObject:nodeID];
10751184
}
1076-
if (storeFailed) {
1077-
storeFailures++;
1078-
MTR_LOG_INFO("Store failed for nodeIndex");
1185+
1186+
if (nodeIndexToStore) {
1187+
if (bulkValuesToStore) {
1188+
bulkValuesToStore[sAttributeCacheNodeIndexKey] = nodeIndexToStore;
1189+
} else {
1190+
BOOL storeFailed = ![self _storeNodeIndex:nodeIndexToStore];
1191+
if (storeFailed) {
1192+
storeFailures++;
1193+
MTR_LOG_INFO("Store failed for nodeIndex");
1194+
}
1195+
}
1196+
}
1197+
1198+
if (bulkValuesToStore) {
1199+
BOOL storeFailed = ![self _bulkStoreAttributeCacheValues:bulkValuesToStore];
1200+
if (storeFailed) {
1201+
storeFailures++;
1202+
MTR_LOG_INFO("Store failed for bulk values count %lu", static_cast<unsigned long>(bulkValuesToStore.count));
1203+
}
10791204
}
10801205

10811206
// In the rare event that store fails, allow all attribute store attempts to go through and prune empty branches at the end altogether.

0 commit comments

Comments
 (0)