Skip to content

Commit 625f5c2

Browse files
jtung-applejtung-apple
authored
[Darwin] MTRDevice should throttle writes to the attribute storage (#33535)
* [Darwin] MTRDevice should throttle writes to the attribute storage * Tune unit test timing to avoid race condition that fails the test when the machine is slow * Additional tuning of unit test timing to avoid timing conditions that fails the test when the machine is slow * Tune base delay to 3 seconds, to account for slow testing machines * Fix unit test * Unit test timing fix and remove debug logging * Fix TSAN issue with test values in unit test
1 parent 57f29a8 commit 625f5c2

16 files changed

+929
-52
lines changed

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

+286-27
Large diffs are not rendered by default.

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

+7
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,8 @@ @implementation MTRDeviceController {
128128

129129
// _serverEndpoints is only touched on the Matter queue.
130130
NSMutableArray<MTRServerEndpoint *> * _serverEndpoints;
131+
132+
MTRDeviceStorageBehaviorConfiguration * _storageBehaviorConfiguration;
131133
}
132134

133135
- (nullable instancetype)initWithParameters:(MTRDeviceControllerAbstractParameters *)parameters error:(NSError * __autoreleasing *)error
@@ -153,6 +155,7 @@ - (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory
153155
otaProviderDelegateQueue:(dispatch_queue_t _Nullable)otaProviderDelegateQueue
154156
uniqueIdentifier:(NSUUID *)uniqueIdentifier
155157
concurrentSubscriptionPoolSize:(NSUInteger)concurrentSubscriptionPoolSize
158+
storageBehaviorConfiguration:(MTRDeviceStorageBehaviorConfiguration *)storageBehaviorConfiguration
156159
{
157160
if (self = [super init]) {
158161
// Make sure our storage is all set up to work as early as possible,
@@ -274,6 +277,8 @@ - (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory
274277
_concurrentSubscriptionPool = [[MTRAsyncWorkQueue alloc] initWithContext:self width:concurrentSubscriptionPoolSize];
275278

276279
_storedFabricIndex = chip::kUndefinedFabricIndex;
280+
281+
_storageBehaviorConfiguration = storageBehaviorConfiguration;
277282
}
278283
return self;
279284
}
@@ -989,6 +994,8 @@ - (MTRDevice *)_setupDeviceForNodeID:(NSNumber *)nodeID prefetchedClusterData:(N
989994
}
990995
}
991996

997+
[deviceToReturn setStorageBehaviorConfiguration:_storageBehaviorConfiguration];
998+
992999
return deviceToReturn;
9931000
}
9941001

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

+4-1
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,7 @@ - (MTRDeviceController * _Nullable)_startDeviceController:(MTRDeviceController *
473473
id<MTROTAProviderDelegate> _Nullable otaProviderDelegate;
474474
dispatch_queue_t _Nullable otaProviderDelegateQueue;
475475
NSUInteger concurrentSubscriptionPoolSize = 0;
476+
MTRDeviceStorageBehaviorConfiguration * storageBehaviorConfiguration = nil;
476477
if ([startupParams isKindOfClass:[MTRDeviceControllerParameters class]]) {
477478
MTRDeviceControllerParameters * params = startupParams;
478479
storageDelegate = params.storageDelegate;
@@ -481,6 +482,7 @@ - (MTRDeviceController * _Nullable)_startDeviceController:(MTRDeviceController *
481482
otaProviderDelegate = params.otaProviderDelegate;
482483
otaProviderDelegateQueue = params.otaProviderDelegateQueue;
483484
concurrentSubscriptionPoolSize = params.concurrentSubscriptionEstablishmentsAllowedOnThread;
485+
storageBehaviorConfiguration = params.storageBehaviorConfiguration;
484486
} else if ([startupParams isKindOfClass:[MTRDeviceControllerStartupParams class]]) {
485487
MTRDeviceControllerStartupParams * params = startupParams;
486488
storageDelegate = nil;
@@ -542,7 +544,8 @@ - (MTRDeviceController * _Nullable)_startDeviceController:(MTRDeviceController *
542544
otaProviderDelegate:otaProviderDelegate
543545
otaProviderDelegateQueue:otaProviderDelegateQueue
544546
uniqueIdentifier:uniqueIdentifier
545-
concurrentSubscriptionPoolSize:concurrentSubscriptionPoolSize];
547+
concurrentSubscriptionPoolSize:concurrentSubscriptionPoolSize
548+
storageBehaviorConfiguration:storageBehaviorConfiguration];
546549
if (controller == nil) {
547550
if (error != nil) {
548551
*error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT];

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

+8
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#import <Matter/MTRDefines.h>
1818

1919
#import <Matter/MTRDeviceControllerStorageDelegate.h>
20+
#import <Matter/MTRDeviceStorageBehaviorConfiguration.h>
2021
#import <Matter/MTROTAProviderDelegate.h>
2122

2223
NS_ASSUME_NONNULL_BEGIN
@@ -85,6 +86,13 @@ MTR_AVAILABLE(ios(17.6), macos(14.6), watchos(10.6), tvos(17.6))
8586
*/
8687
@property (nonatomic, assign) NSUInteger concurrentSubscriptionEstablishmentsAllowedOnThread MTR_NEWLY_AVAILABLE;
8788

89+
/**
90+
* Sets the storage behavior configuration - see MTRDeviceStorageBehaviorConfiguration.h for details
91+
*
92+
* If this value is nil, a default storage behavior configuration will be used.
93+
*/
94+
@property (nonatomic, copy, nullable) MTRDeviceStorageBehaviorConfiguration * storageBehaviorConfiguration;
95+
8896
@end
8997

9098
MTR_AVAILABLE(ios(17.6), macos(14.6), watchos(10.6), tvos(17.6))

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

+3-1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#import "MTRBaseDevice.h"
3333
#import "MTRDeviceController.h"
3434
#import "MTRDeviceControllerDataStore.h"
35+
#import "MTRDeviceStorageBehaviorConfiguration.h"
3536

3637
#import <Matter/MTRDefines.h>
3738
#import <Matter/MTRDeviceControllerStartupParams.h>
@@ -113,7 +114,8 @@ NS_ASSUME_NONNULL_BEGIN
113114
otaProviderDelegate:(id<MTROTAProviderDelegate> _Nullable)otaProviderDelegate
114115
otaProviderDelegateQueue:(dispatch_queue_t _Nullable)otaProviderDelegateQueue
115116
uniqueIdentifier:(NSUUID *)uniqueIdentifier
116-
concurrentSubscriptionPoolSize:(NSUInteger)concurrentSubscriptionPoolSize;
117+
concurrentSubscriptionPoolSize:(NSUInteger)concurrentSubscriptionPoolSize
118+
storageBehaviorConfiguration:(MTRDeviceStorageBehaviorConfiguration *)storageBehaviorConfiguration;
117119

118120
/**
119121
* Check whether this controller is running on the given fabric, as represented

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

+98
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/**
2+
* Copyright (c) 2024 Project CHIP Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#import <Foundation/Foundation.h>
18+
#import <Matter/MTRDefines.h>
19+
20+
NS_ASSUME_NONNULL_BEGIN
21+
22+
/**
23+
* Class that configures how MTRDevice objects persist its attributes to storage, so as to not
24+
* overwhelm the underlying storage system.
25+
*/
26+
MTR_NEWLY_AVAILABLE
27+
@interface MTRDeviceStorageBehaviorConfiguration : NSObject <NSCopying>
28+
29+
/**
30+
* Create configuration with a default set of values. See description below for details.
31+
*/
32+
+ (instancetype)configurationWithDefaultStorageBehavior;
33+
34+
/**
35+
* Create configuration that disables storage behavior optimizations.
36+
*/
37+
+ (instancetype)configurationWithStorageBehaviorOptimizationDisabled;
38+
39+
/**
40+
* Create configuration with specified values. See description below for details, and the list of
41+
* properties below for valid ranges of these values.
42+
*/
43+
+ (instancetype)configurationWithReportToPersistenceDelayTime:(NSTimeInterval)reportToPersistenceDelayTime
44+
reportToPersistenceDelayTimeMax:(NSTimeInterval)reportToPersistenceDelayTimeMax
45+
recentReportTimesMaxCount:(NSUInteger)recentReportTimesMaxCount
46+
timeBetweenReportsTooShortThreshold:(NSTimeInterval)timeBetweenReportsTooShortThreshold
47+
timeBetweenReportsTooShortMinThreshold:(NSTimeInterval)timeBetweenReportsTooShortMinThreshold
48+
reportToPersistenceDelayMaxMultiplier:(double)reportToPersistenceDelayMaxMultiplier
49+
deviceReportingExcessivelyIntervalThreshold:(NSTimeInterval)deviceReportingExcessivelyIntervalThreshold;
50+
51+
/**
52+
* Storage behavior with values in the allowed range:
53+
*
54+
* Each time a report comes in, MTRDevice will wait reportToPersistDelayTime before persisting the
55+
* changes to storage. If another report comes in during this internal, MTRDevice will wait another
56+
* reportToPersistDelayTime interval, until reportToPersistDelayTimeMax is reached, at which
57+
* point all the changes so far will be written to storage.
58+
*
59+
* MTRDevice will also track recentReportTimesMaxCount number of report times. If the running
60+
* average time between reports dips below timeBetweenReportsTooShortThreshold, a portion of the
61+
* reportToPersistenceDelayMaxMultiplier will be applied to both the reportToPersistenceDelayTime
62+
* and reportToPersistenceDelayTimeMax. The multiplier will reach the max when the average time
63+
* between reports reach timeBetweenReportsTooShortMinThreshold.
64+
*
65+
* When the running average time between reports dips below timeBetweenReportsTooShortMinThreshold
66+
* for the first time, the time will be noted. If the device remains in this state for longer than
67+
* deviceReportingExcessivelyIntervalThreshold, persistence will stop until the average time between
68+
* reports go back above timeBetweenReportsTooShortMinThreshold.
69+
*/
70+
71+
/**
72+
* If disableStorageBehaviorOptimization is set to YES, then all the waiting mechanism as described above
73+
* is disabled.
74+
*/
75+
@property (nonatomic, assign) BOOL disableStorageBehaviorOptimization;
76+
77+
/**
78+
* If any of these properties are set to be out of the documented limits, these default values will
79+
* be used to replace all of them:
80+
*
81+
* reportToPersistenceDelayTimeDefault (15)
82+
* reportToPersistenceDelayTimeMaxDefault (20 * kReportToPersistenceDelayTimeDefault)
83+
* recentReportTimesMaxCountDefault (12)
84+
* timeBetweenReportsTooShortThresholdDefault (15)
85+
* timeBetweenReportsTooShortMinThresholdDefault (5)
86+
* reportToPersistenceDelayMaxMultiplierDefault (10)
87+
* deviceReportingExcessivelyIntervalThresholdDefault (5 * 60)
88+
*/
89+
@property (nonatomic, assign) NSTimeInterval reportToPersistenceDelayTime; /* must be > 0 */
90+
@property (nonatomic, assign) NSTimeInterval reportToPersistenceDelayTimeMax; /* must be larger than reportToPersistenceDelayTime */
91+
@property (nonatomic, assign) NSUInteger recentReportTimesMaxCount; /* must be >= 2 */
92+
@property (nonatomic, assign) NSTimeInterval timeBetweenReportsTooShortThreshold; /* must be > 0 */
93+
@property (nonatomic, assign) NSTimeInterval timeBetweenReportsTooShortMinThreshold; /* must be > 0 and smaller than timeBetweenReportsTooShortThreshold */
94+
@property (nonatomic, assign) double reportToPersistenceDelayMaxMultiplier; /* must be > 1 */
95+
@property (nonatomic, assign) NSTimeInterval deviceReportingExcessivelyIntervalThreshold; /* must be > 0 */
96+
@end
97+
98+
NS_ASSUME_NONNULL_END

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

+105
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/**
2+
* Copyright (c) 2024 Project CHIP Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#import "MTRDeviceStorageBehaviorConfiguration.h"
18+
19+
#import "MTRLogging_Internal.h"
20+
21+
#define kReportToPersistenceDelayTimeDefault (15)
22+
#define kReportToPersistenceDelayTimeMaxDefault (20 * kReportToPersistenceDelayTimeDefault)
23+
#define kRecentReportTimesMaxCountDefault (12)
24+
#define kTimeBetweenReportsTooShortThresholdDefault (15)
25+
#define kTimeBetweenReportsTooShortMinThresholdDefault (5)
26+
#define kReportToPersistenceDelayMaxMultiplierDefault (10)
27+
#define kDeviceReportingExcessivelyIntervalThresholdDefault (5 * 60)
28+
29+
@implementation MTRDeviceStorageBehaviorConfiguration
30+
31+
+ (instancetype)configurationWithReportToPersistenceDelayTime:(NSTimeInterval)reportToPersistenceDelayTime
32+
reportToPersistenceDelayTimeMax:(NSTimeInterval)reportToPersistenceDelayTimeMax
33+
recentReportTimesMaxCount:(NSUInteger)recentReportTimesMaxCount
34+
timeBetweenReportsTooShortThreshold:(NSTimeInterval)timeBetweenReportsTooShortThreshold
35+
timeBetweenReportsTooShortMinThreshold:(NSTimeInterval)timeBetweenReportsTooShortMinThreshold
36+
reportToPersistenceDelayMaxMultiplier:(double)reportToPersistenceDelayMaxMultiplier
37+
deviceReportingExcessivelyIntervalThreshold:(NSTimeInterval)deviceReportingExcessivelyIntervalThreshold
38+
{
39+
auto newConfiguration = [[MTRDeviceStorageBehaviorConfiguration alloc] init];
40+
newConfiguration.reportToPersistenceDelayTime = reportToPersistenceDelayTime;
41+
newConfiguration.reportToPersistenceDelayTimeMax = reportToPersistenceDelayTimeMax;
42+
newConfiguration.recentReportTimesMaxCount = recentReportTimesMaxCount;
43+
newConfiguration.timeBetweenReportsTooShortThreshold = timeBetweenReportsTooShortThreshold;
44+
newConfiguration.timeBetweenReportsTooShortMinThreshold = timeBetweenReportsTooShortMinThreshold;
45+
newConfiguration.reportToPersistenceDelayMaxMultiplier = reportToPersistenceDelayMaxMultiplier;
46+
newConfiguration.deviceReportingExcessivelyIntervalThreshold = deviceReportingExcessivelyIntervalThreshold;
47+
48+
return newConfiguration;
49+
}
50+
51+
+ (instancetype)configurationWithDefaultStorageBehavior
52+
{
53+
auto newConfiguration = [[MTRDeviceStorageBehaviorConfiguration alloc] init];
54+
[newConfiguration checkValuesAndResetToDefaultIfNecessary];
55+
return newConfiguration;
56+
}
57+
58+
+ (instancetype)configurationWithStorageBehaviorOptimizationDisabled
59+
{
60+
auto newConfiguration = [[MTRDeviceStorageBehaviorConfiguration alloc] init];
61+
newConfiguration.disableStorageBehaviorOptimization = YES;
62+
return newConfiguration;
63+
}
64+
65+
- (NSString *)description
66+
{
67+
return [NSString stringWithFormat:@"<MTRDeviceStorageBehaviorConfiguration(%p): disabled %s reportToPersistenceDelayTime %lf reportToPersistenceDelayTimeMax %lf recentReportTimesMaxCount %lu timeBetweenReportsTooShortThreshold %lf timeBetweenReportsTooShortMinThreshold %lf reportToPersistenceDelayMaxMultiplier %lf deviceReportingExcessivelyIntervalThreshold %lf", self, _disableStorageBehaviorOptimization ? "YES" : "NO", _reportToPersistenceDelayTime, _reportToPersistenceDelayTimeMax, static_cast<unsigned long>(_recentReportTimesMaxCount), _timeBetweenReportsTooShortThreshold, _timeBetweenReportsTooShortMinThreshold, _reportToPersistenceDelayMaxMultiplier, _deviceReportingExcessivelyIntervalThreshold];
68+
}
69+
70+
- (void)checkValuesAndResetToDefaultIfNecessary
71+
{
72+
if (_disableStorageBehaviorOptimization) {
73+
return;
74+
}
75+
76+
// Sanity check all the values, and if any is out of range, reset to default values
77+
if ((_reportToPersistenceDelayTime <= 0) || (_reportToPersistenceDelayTimeMax <= 0) || (_reportToPersistenceDelayTimeMax < _reportToPersistenceDelayTime) || (_recentReportTimesMaxCount < 2) || (_timeBetweenReportsTooShortThreshold <= 0) || (_timeBetweenReportsTooShortMinThreshold <= 0) || (_timeBetweenReportsTooShortMinThreshold > _timeBetweenReportsTooShortThreshold) || (_reportToPersistenceDelayMaxMultiplier <= 1) || (_deviceReportingExcessivelyIntervalThreshold <= 0)) {
78+
MTR_LOG_ERROR("%@ storage behavior: MTRDeviceStorageBehaviorConfiguration values out of bounds - resetting to default", self);
79+
80+
_reportToPersistenceDelayTime = kReportToPersistenceDelayTimeDefault;
81+
_reportToPersistenceDelayTimeMax = kReportToPersistenceDelayTimeMaxDefault;
82+
_recentReportTimesMaxCount = kRecentReportTimesMaxCountDefault;
83+
_timeBetweenReportsTooShortThreshold = kTimeBetweenReportsTooShortThresholdDefault;
84+
_timeBetweenReportsTooShortMinThreshold = kTimeBetweenReportsTooShortMinThresholdDefault;
85+
_reportToPersistenceDelayMaxMultiplier = kReportToPersistenceDelayMaxMultiplierDefault;
86+
_deviceReportingExcessivelyIntervalThreshold = kDeviceReportingExcessivelyIntervalThresholdDefault;
87+
}
88+
}
89+
90+
- (id)copyWithZone:(NSZone *)zone
91+
{
92+
auto newConfiguration = [[MTRDeviceStorageBehaviorConfiguration alloc] init];
93+
newConfiguration.disableStorageBehaviorOptimization = _disableStorageBehaviorOptimization;
94+
newConfiguration.reportToPersistenceDelayTime = _reportToPersistenceDelayTime;
95+
newConfiguration.reportToPersistenceDelayTimeMax = _reportToPersistenceDelayTimeMax;
96+
newConfiguration.recentReportTimesMaxCount = _recentReportTimesMaxCount;
97+
newConfiguration.timeBetweenReportsTooShortThreshold = _timeBetweenReportsTooShortThreshold;
98+
newConfiguration.timeBetweenReportsTooShortMinThreshold = _timeBetweenReportsTooShortMinThreshold;
99+
newConfiguration.reportToPersistenceDelayMaxMultiplier = _reportToPersistenceDelayMaxMultiplier;
100+
newConfiguration.deviceReportingExcessivelyIntervalThreshold = _deviceReportingExcessivelyIntervalThreshold;
101+
102+
return newConfiguration;
103+
}
104+
105+
@end

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

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* Copyright (c) 2024 Project CHIP Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#import "MTRDeviceStorageBehaviorConfiguration.h"
18+
19+
@interface MTRDeviceStorageBehaviorConfiguration ()
20+
- (void)checkValuesAndResetToDefaultIfNecessary;
21+
@end

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

+3
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
#import "MTRAsyncWorkQueue.h"
2323
#import "MTRDefines_Internal.h"
24+
#import "MTRDeviceStorageBehaviorConfiguration_Internal.h"
2425

2526
NS_ASSUME_NONNULL_BEGIN
2627

@@ -113,6 +114,8 @@ MTR_TESTABLE
113114
- (NSUInteger)unitTestAttributeCount;
114115
#endif
115116

117+
- (void)setStorageBehaviorConfiguration:(MTRDeviceStorageBehaviorConfiguration *)storageBehaviorConfiguration;
118+
116119
@end
117120

118121
#pragma mark - Utility for clamping numbers

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

+1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
#import <Matter/MTRDeviceControllerParameters.h>
4848
#import <Matter/MTRDeviceControllerStartupParams.h>
4949
#import <Matter/MTRDeviceControllerStorageDelegate.h>
50+
#import <Matter/MTRDeviceStorageBehaviorConfiguration.h>
5051
#import <Matter/MTRDeviceTypeRevision.h>
5152
#import <Matter/MTRDiagnosticLogsType.h>
5253
#import <Matter/MTRError.h>

0 commit comments

Comments
 (0)