Skip to content

Commit 28b896a

Browse files
Provide an estimate of subscription latency in MTRDevice. (#33490)
The idea is to track how long subscriptions took in the past and surface an estimate based on that to the API consumer.
1 parent 3b648b6 commit 28b896a

7 files changed

+174
-1
lines changed

src/darwin/Framework/CHIP/MTRDevice.h

+8
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,14 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
101101
*/
102102
@property (nonatomic, readonly, copy) NSNumber * nodeID NS_REFINED_FOR_SWIFT MTR_AVAILABLE(ios(17.4), macos(14.4), watchos(10.4), tvos(17.4));
103103

104+
/**
105+
* An estimate of how much time is likely to elapse between setDelegate being
106+
* called and the current device state (attributes, stored events) being known.
107+
*
108+
* nil if no such estimate is available. Otherwise, the NSNumber stores an NSTimeInterval.
109+
*/
110+
@property (nonatomic, readonly, nullable, copy) NSNumber * estimatedSubscriptionLatency MTR_NEWLY_AVAILABLE;
111+
104112
/**
105113
* Set the delegate to receive asynchronous callbacks about the device.
106114
*

src/darwin/Framework/CHIP/MTRDevice.mm

+62
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,7 @@ @implementation MTRDeviceClusterData {
213213

214214
static NSString * const sDataVersionKey = @"dataVersion";
215215
static NSString * const sAttributesKey = @"attributes";
216+
static NSString * const sLastInitialSubscribeLatencyKey = @"lastInitialSubscribeLatency";
216217

217218
- (void)storeValue:(MTRDeviceDataValueDictionary _Nullable)value forAttribute:(NSNumber *)attribute
218219
{
@@ -414,6 +415,10 @@ @implementation MTRDevice {
414415
// 2. OnResubscriptionNeeded is called
415416
// 3. Subscription reset (including when getSessionForNode fails)
416417
MTRAsyncWorkCompletionBlock _subscriptionPoolWorkCompletionBlock;
418+
419+
// Tracking of initial subscribe latency. When _initialSubscribeStart is
420+
// nil, we are not tracking the latency.
421+
NSDate * _Nullable _initialSubscribeStart;
417422
}
418423

419424
- (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller
@@ -705,6 +710,7 @@ - (void)setDelegate:(id<MTRDeviceDelegate>)delegate queue:(dispatch_queue_t)queu
705710
}
706711

707712
if (setUpSubscription) {
713+
_initialSubscribeStart = [NSDate now];
708714
if ([self _deviceUsesThread]) {
709715
[self _scheduleSubscriptionPoolWork:^{
710716
std::lock_guard lock(self->_lock);
@@ -954,6 +960,18 @@ - (void)_handleSubscriptionEstablished
954960
// No need to monitor connectivity after subscription establishment
955961
[self _stopConnectivityMonitoring];
956962

963+
auto initialSubscribeStart = _initialSubscribeStart;
964+
// We no longer need to track subscribe latency for this device.
965+
_initialSubscribeStart = nil;
966+
967+
if (initialSubscribeStart != nil) {
968+
// We want time interval from initialSubscribeStart to now, not the other
969+
// way around.
970+
NSTimeInterval subscriptionLatency = -[initialSubscribeStart timeIntervalSinceNow];
971+
_estimatedSubscriptionLatency = @(subscriptionLatency);
972+
[self _storePersistedDeviceData];
973+
}
974+
957975
os_unfair_lock_unlock(&self->_lock);
958976

959977
os_unfair_lock_lock(&self->_timeSyncLock);
@@ -2884,6 +2902,50 @@ - (void)setPersistedClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceCluster
28842902
}
28852903
}
28862904

2905+
- (void)_setLastInitialSubscribeLatency:(id)latency
2906+
{
2907+
os_unfair_lock_assert_owner(&self->_lock);
2908+
2909+
if (![latency isKindOfClass:NSNumber.class]) {
2910+
// Unexpected value of some sort; just ignore it.
2911+
return;
2912+
}
2913+
2914+
_estimatedSubscriptionLatency = latency;
2915+
}
2916+
2917+
- (void)setPersistedDeviceData:(NSDictionary<NSString *, id> *)data
2918+
{
2919+
MTR_LOG_INFO("%@ setPersistedDeviceData: %@", self, data);
2920+
2921+
std::lock_guard lock(_lock);
2922+
2923+
// For now the only data we care about is our initial subscribe latency.
2924+
id initialSubscribeLatency = data[sLastInitialSubscribeLatencyKey];
2925+
if (initialSubscribeLatency != nil) {
2926+
[self _setLastInitialSubscribeLatency:initialSubscribeLatency];
2927+
}
2928+
}
2929+
2930+
- (void)_storePersistedDeviceData
2931+
{
2932+
os_unfair_lock_assert_owner(&self->_lock);
2933+
2934+
auto datastore = _deviceController.controllerDataStore;
2935+
if (datastore == nil) {
2936+
// No way to store.
2937+
return;
2938+
}
2939+
2940+
// For now the only data we have is our initial subscribe latency.
2941+
NSMutableDictionary<NSString *, id> * data = [NSMutableDictionary dictionary];
2942+
if (_estimatedSubscriptionLatency != nil) {
2943+
data[sLastInitialSubscribeLatencyKey] = _estimatedSubscriptionLatency;
2944+
}
2945+
2946+
[datastore storeDeviceData:[data copy] forNodeID:self.nodeID];
2947+
}
2948+
28872949
- (BOOL)deviceCachePrimed
28882950
{
28892951
std::lock_guard lock(_lock);

src/darwin/Framework/CHIP/MTRDeviceController.mm

+9-1
Original file line numberDiff line numberDiff line change
@@ -966,7 +966,7 @@ - (MTRDevice *)_setupDeviceForNodeID:(NSNumber *)nodeID prefetchedClusterData:(N
966966
if (prefetchedClusterData.count) {
967967
[deviceToReturn setPersistedClusterData:prefetchedClusterData];
968968
}
969-
} else {
969+
} else if (_controllerDataStore) {
970970
// Load persisted cluster data if they exist.
971971
NSDictionary * clusterData = [_controllerDataStore getStoredClusterDataForNodeID:nodeID];
972972
MTR_LOG_INFO("Loaded %lu cluster data from storage for %@", static_cast<unsigned long>(clusterData.count), deviceToReturn);
@@ -975,6 +975,14 @@ - (MTRDevice *)_setupDeviceForNodeID:(NSNumber *)nodeID prefetchedClusterData:(N
975975
}
976976
}
977977

978+
// TODO: Figure out how to get the device data as part of our bulk-read bits.
979+
if (_controllerDataStore) {
980+
auto * deviceData = [_controllerDataStore getStoredDeviceDataForNodeID:nodeID];
981+
if (deviceData.count) {
982+
[deviceToReturn setPersistedDeviceData:deviceData];
983+
}
984+
}
985+
978986
return deviceToReturn;
979987
}
980988

src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h

+10
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,16 @@ typedef void (^MTRDeviceControllerDataStoreClusterDataHandler)(NSDictionary<NSNu
7878
- (void)clearStoredClusterDataForNodeID:(NSNumber *)nodeID;
7979
- (void)clearAllStoredClusterData;
8080

81+
/**
82+
* Storage for miscellaneous MTRDevice data that only needs to be stored
83+
* locally.
84+
*
85+
* storeDeviceData uses data and nodeID async without copying, so callers should
86+
* not modify the passed-in dictionary or nodeID.
87+
*/
88+
- (nullable NSDictionary<NSString *, id> *)getStoredDeviceDataForNodeID:(NSNumber *)nodeID;
89+
- (void)storeDeviceData:(NSDictionary<NSString *, id> *)data forNodeID:(NSNumber *)nodeID;
90+
8191
@end
8292

8393
NS_ASSUME_NONNULL_END

src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm

+59
Original file line numberDiff line numberDiff line change
@@ -1020,6 +1020,65 @@ - (void)storeClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *>
10201020
});
10211021
}
10221022

1023+
static NSString * sDeviceDataKeyPrefix = @"deviceData";
1024+
1025+
- (NSString *)_deviceDataKeyForNodeID:(NSNumber *)nodeID
1026+
{
1027+
return [sDeviceDataKeyPrefix stringByAppendingFormat:@":0x%016llX", nodeID.unsignedLongLongValue];
1028+
}
1029+
1030+
- (nullable NSDictionary<NSString *, id> *)getStoredDeviceDataForNodeID:(NSNumber *)nodeID
1031+
{
1032+
__block NSDictionary<NSString *, id> * deviceData = nil;
1033+
dispatch_sync(_storageDelegateQueue, ^{
1034+
MTRDeviceController * controller = self->_controller;
1035+
VerifyOrReturn(controller != nil); // No way to call delegate without controller.
1036+
1037+
id data;
1038+
@autoreleasepool {
1039+
data = [self->_storageDelegate controller:controller
1040+
valueForKey:[self _deviceDataKeyForNodeID:nodeID]
1041+
securityLevel:MTRStorageSecurityLevelSecure
1042+
sharingType:MTRStorageSharingTypeNotShared];
1043+
}
1044+
if (data == nil) {
1045+
return;
1046+
}
1047+
1048+
if (![data isKindOfClass:NSDictionary.class]) {
1049+
return;
1050+
}
1051+
1052+
// Check that all the keys are in fact strings.
1053+
NSDictionary * dictionary = data;
1054+
for (id key in dictionary) {
1055+
if (![key isKindOfClass:NSString.class]) {
1056+
return;
1057+
}
1058+
}
1059+
1060+
// We can't do value type verification; our API consumer will need
1061+
// to do that.
1062+
deviceData = dictionary;
1063+
});
1064+
return deviceData;
1065+
}
1066+
1067+
- (void)storeDeviceData:(NSDictionary<NSString *, id> *)data forNodeID:(NSNumber *)nodeID
1068+
{
1069+
dispatch_async(_storageDelegateQueue, ^{
1070+
MTRDeviceController * controller = self->_controller;
1071+
VerifyOrReturn(controller != nil); // No way to call delegate without controller.
1072+
1073+
// Ignore store failures, since they are not actionable for us here.
1074+
[self->_storageDelegate controller:controller
1075+
storeValue:data
1076+
forKey:[self _deviceDataKeyForNodeID:nodeID]
1077+
securityLevel:MTRStorageSecurityLevelSecure
1078+
sharingType:MTRStorageSharingTypeNotShared];
1079+
});
1080+
}
1081+
10231082
@end
10241083

10251084
@implementation MTRCASESessionResumptionInfo

src/darwin/Framework/CHIP/MTRDevice_Internal.h

+3
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ MTR_TESTABLE
8686
// Contains data version information and attribute values.
8787
- (void)setPersistedClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *)clusterData;
8888

89+
// Method to insert persisted data that pertains to the whole device.
90+
- (void)setPersistedDeviceData:(NSDictionary<NSString *, id> *)data;
91+
8992
#ifdef DEBUG
9093
- (NSUInteger)unitTestAttributeCount;
9194
#endif

src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m

+23
Original file line numberDiff line numberDiff line change
@@ -1362,10 +1362,18 @@ - (void)doDataStoreMTRDeviceTestWithStorageDelegate:(id<MTRDeviceControllerStora
13621362
[subscriptionExpectation fulfill];
13631363
};
13641364

1365+
// Verify that initially (before we have ever subscribed while using this
1366+
// datastore) the device has no estimate for subscription latency.
1367+
XCTAssertNil(device.estimatedSubscriptionLatency);
1368+
1369+
__auto_type * beforeSetDelegate = [NSDate now];
1370+
13651371
[device setDelegate:delegate queue:queue];
13661372

13671373
[self waitForExpectations:@[ subscriptionExpectation ] timeout:60];
13681374

1375+
__auto_type * afterInitialSubscription = [NSDate now];
1376+
13691377
NSUInteger dataStoreValuesCount = 0;
13701378
NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> * dataStoreClusterData = [controller.controllerDataStore getStoredClusterDataForNodeID:deviceID];
13711379
for (MTRClusterPath * path in dataStoreClusterData) {
@@ -1400,6 +1408,21 @@ - (void)doDataStoreMTRDeviceTestWithStorageDelegate:(id<MTRDeviceControllerStora
14001408
double storedAttributeDifferFromMTRDevicePercentage = storedAttributeDifferFromMTRDeviceCount * 100.0 / dataStoreValuesCount;
14011409
XCTAssertTrue(storedAttributeDifferFromMTRDevicePercentage < 10.0);
14021410

1411+
// Check that the new device has an estimated subscription latency.
1412+
XCTAssertNotNil(device.estimatedSubscriptionLatency);
1413+
1414+
// Check that this estimate is positive, since subscribing must have taken
1415+
// some time.
1416+
XCTAssertGreaterThan(device.estimatedSubscriptionLatency.doubleValue, 0);
1417+
1418+
// Check that this estimate is no larger than the measured latency observed
1419+
// above. Unfortunately, We measure our observed latency to report end, not
1420+
// to the immediately following internal subscription established
1421+
// notification, so in fact our measured value can end up shorter than the
1422+
// estimated latency the device has. Add some slop to handle that.
1423+
const NSTimeInterval timingSlopInSeconds = 0.1;
1424+
XCTAssertLessThanOrEqual(device.estimatedSubscriptionLatency.doubleValue, [afterInitialSubscription timeIntervalSinceDate:beforeSetDelegate] + timingSlopInSeconds);
1425+
14031426
// Now set up new delegate for the new device and verify that once subscription reestablishes, the data version filter loaded from storage will work
14041427
__auto_type * newDelegate = [[MTRDeviceTestDelegate alloc] init];
14051428

0 commit comments

Comments
 (0)