Skip to content

Commit

Permalink
Darwin: Consolidate MTRAsyncWorkQueue headers
Browse files Browse the repository at this point in the history
The internal header is no longer needed because MTRAsyncWorkQueue is entirely
private to the project now. Also tidy up duplicate / batching logic a little.
  • Loading branch information
ksperling-apple committed Sep 26, 2023
1 parent db0dc53 commit d4101be
Show file tree
Hide file tree
Showing 6 changed files with 123 additions and 141 deletions.
94 changes: 91 additions & 3 deletions src/darwin/Framework/CHIP/MTRAsyncWorkQueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,54 @@ typedef NS_ENUM(NSInteger, MTRAsyncWorkOutcome) {
/// being cancelled).
typedef BOOL (^MTRAsyncWorkCompletionBlock)(MTRAsyncWorkOutcome outcome);

/// An optional handler that controls batching of MTRAsyncWorkItem.
///
/// When a work item is dequeued to run, if it is of a type that can be
/// combined with similar work items in a batch, this facility gives the
/// provides an opportunity to coalesce and merge work items.
///
/// The batching handler is called by the work queue when all of the following
/// are true:
/// 1) A work item that is batchable is about to be executed for the first time
/// 2) The next work item in the queue is also batchable
/// 3) The two work items have identical batching ids
///
/// The handler will be passed the opaque data of the two work items:
/// `opaqueDataCurrent` is the data of the item about to be executed and
/// `opaqueDataNext` is the data for the next item.
///
/// The handler is expected to mutate the data as needed to achieve batching.
///
/// If after the data mutations opaqueDataNext no longer requires any work, the
/// handler should set `fullyMerged` to YES to indicate that the next item can
/// be dropped from the queue. In this case, the handler may be called again to
/// possibly also batch the work item after the one that was dropped.
///
/// @see MTRAsyncWorkItem
typedef void (^MTRAsyncWorkBatchingHandler)(id opaqueDataCurrent, id opaqueDataNext, BOOL * fullyMerged);

/// An optional handler than enables duplicate checking for MTRAsyncWorkItem.
///
/// The duplicate check handler is called when the client wishes to check
/// whether a new candidate work item is a duplicate of an existing queued
/// item, so that the client may decide to not enqueue the duplicate work.
/// Duplicate checking is performed in reverse queue order, i.e. more
/// recently enqueued items will be checked first.
///
/// The handler will be passed the opaque data of the candidate work item.
///
/// If the handler determines the data is indeed duplicate work, it should
/// set `stop` to YES, and set `isDuplicate` to YES.
///
/// If the handler determines the data is not duplicate work, it should set
/// `stop` to YES, and set `isDuplicate` to NO.
///
/// If the handler is unable to determine if the data is duplicate work, it
/// should set `stop` to NO; the value of `isDuplicate` will be ignored.
///
/// @see MTRAsyncWorkItem
typedef void (^MTRAsyncWorkDuplicateCheckHandler)(id opaqueItemData, BOOL * isDuplicate, BOOL * stop);

/// A unit of work that can be run on a `MTRAsyncWorkQueue`.
///
/// A work item can be configured with a number of hander blocks called by the
Expand All @@ -50,7 +98,9 @@ MTR_TESTABLE
/// Creates a work item that will run on the specified dispatch queue.
- (instancetype)initWithQueue:(dispatch_queue_t)queue;

/// Called by the work queue to start this work item
/// Called by the work queue to start this work item.
///
/// Will be called on the dispatch queue associated with this item.
///
/// This handler block must, synchronously or asynchronously from any thread,
/// call the provided completion block exactly once. Passing an outcome of
Expand All @@ -63,10 +113,38 @@ MTR_TESTABLE
@property (nonatomic, strong, nullable) void (^readyHandler)
(ContextType context, NSInteger retryCount, MTRAsyncWorkCompletionBlock completion);

/// Called by the work queue to cancel the work item. The work item may or may
/// not have been started already.
/// Called by the work queue to cancel the work item.
///
/// Will be called on the dispatch queue associated with this item.
/// The work item may or may not have been started already.
@property (nonatomic, strong, nullable) void (^cancelHandler)(void);

@property (nonatomic, readonly) NSUInteger batchingID;
@property (nonatomic, readonly) id batchableData;
@property (nonatomic, readonly) MTRAsyncWorkBatchingHandler batchingHandler;

/// Sets the batching handler and associated data for this work item.
///
/// Note: This handler is NOT called on the dispatch queue associated with
/// this work item. Thread-safety is managed by the work queue internally.
///
/// @see MTRAsyncWorkBatchingHandler
- (void)setBatchingID:(NSUInteger)opaqueBatchingID
data:(id)opaqueBatchableData
handler:(MTRAsyncWorkBatchingHandler)batchingHandler;

@property (nonatomic, readonly) NSUInteger duplicateTypeID;
@property (nonatomic, readonly) MTRAsyncWorkDuplicateCheckHandler duplicateCheckHandler;

/// Sets the duplicate check type and handler for this work item.
///
/// Note: This handler is NOT called on the dispatch queue associated with
/// this work item. Thread-safety is managed by the work queue internally.
///
/// @see MTRAsyncWorkDuplicateCheckHandler
- (void)setDuplicateTypeID:(NSUInteger)opaqueDuplicateTypeID
handler:(MTRAsyncWorkDuplicateCheckHandler)duplicateCheckHandler;

@end

/// A serial one-at-a-time queue for performing asynchronous work items.
Expand Down Expand Up @@ -100,6 +178,16 @@ MTR_TESTABLE
/// re-used.
- (void)enqueueWorkItem:(MTRAsyncWorkItem<ContextType> *)item;

/// Checks whether the queue already contains a work item matching the provided
/// details. A client may call this method to avoid enqueueing duplicate work.
///
/// This method will call the duplicate check handler for all work items
/// matching the duplicate type ID, starting from the last item in the queue
///
/// @see MTRAsyncWorkDuplicateCheckHandler
- (BOOL)hasDuplicateForTypeID:(NSUInteger)opaqueDuplicateTypeID
workItemData:(id)opaqueWorkItemData;

/// Cancels and removes all work items.
- (void)invalidate;
@end
Expand Down
34 changes: 18 additions & 16 deletions src/darwin/Framework/CHIP/MTRAsyncWorkQueue.mm
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* limitations under the License.
*/

#import "MTRAsyncWorkQueue_Internal.h"
#import "MTRAsyncWorkQueue.h"
#import "MTRDefines_Internal.h"
#import "MTRLogging_Internal.h"

Expand Down Expand Up @@ -131,17 +131,17 @@ - (void)cancel

- (void)setBatchingID:(NSUInteger)opaqueBatchingID data:(id)opaqueBatchableData handler:(MTRAsyncWorkBatchingHandler)batchingHandler
{
NSParameterAssert(batchingHandler);
[self assertMutable];
_batchable = YES;
_batchingID = opaqueBatchingID;
_batchableData = opaqueBatchableData;
_batchingHandler = batchingHandler;
}

- (void)setDuplicateTypeID:(NSUInteger)opaqueDuplicateTypeID handler:(MTRAsyncWorkDuplicateCheckHandler)duplicateCheckHandler
{
NSParameterAssert(duplicateCheckHandler);
[self assertMutable];
_supportsDuplicateCheck = YES;
_duplicateTypeID = opaqueDuplicateTypeID;
_duplicateCheckHandler = duplicateCheckHandler;
}
Expand Down Expand Up @@ -241,19 +241,18 @@ - (void)_callNextReadyWorkItem
MTRAsyncWorkItem * workItem = _items.firstObject;

// Check if batching is possible or needed. Only ask work item to batch once for simplicity
if (workItem.batchable && workItem.batchingHandler && (workItem.retryCount == 0)) {
auto batchingHandler = workItem.batchingHandler;
if (batchingHandler && workItem.retryCount == 0) {
while (_items.count >= 2) {
MTRAsyncWorkItem * nextWorkItem = _items[1];
if (!nextWorkItem.batchable || (nextWorkItem.batchingID != workItem.batchingID)) {
// next item is not eligible to merge with this one
break;
if (!nextWorkItem.batchingHandler || nextWorkItem.batchingID != workItem.batchingID) {
break; // next item is not eligible to merge with this one
}

BOOL fullyMerged = NO;
workItem.batchingHandler(workItem.batchableData, nextWorkItem.batchableData, &fullyMerged);
batchingHandler(workItem.batchableData, nextWorkItem.batchableData, &fullyMerged);
if (!fullyMerged) {
// We can't remove the next work item, so we can't merge anything else into this one.
break;
break; // not removing the next item, so we can't merge anything else
}

[_items removeObjectAtIndex:1];
Expand All @@ -276,22 +275,25 @@ - (void)_callNextReadyWorkItem
}];
}

- (BOOL)isDuplicateForTypeID:(NSUInteger)opaqueDuplicateTypeID workItemData:(id)opaqueWorkItemData
- (BOOL)hasDuplicateForTypeID:(NSUInteger)opaqueDuplicateTypeID workItemData:(id)opaqueWorkItemData
{
BOOL isDuplicate = NO;
BOOL hasDuplicate = NO;
os_unfair_lock_lock(&_lock);
// Start from the last item
for (MTRAsyncWorkItem * item in [_items reverseObjectEnumerator]) {
BOOL stop = NO;
if (item.supportsDuplicateCheck && (item.duplicateTypeID == opaqueDuplicateTypeID) && item.duplicateCheckHandler) {
item.duplicateCheckHandler(opaqueWorkItemData, &isDuplicate, &stop);
auto duplicateCheckHandler = item.duplicateCheckHandler;
if (duplicateCheckHandler && item.duplicateTypeID == opaqueDuplicateTypeID) {
BOOL stop = NO;
BOOL isDuplicate = NO;
duplicateCheckHandler(opaqueWorkItemData, &isDuplicate, &stop);
if (stop) {
hasDuplicate = isDuplicate;
break;
}
}
}
os_unfair_lock_unlock(&_lock);
return isDuplicate;
return hasDuplicate;
}

@end
102 changes: 0 additions & 102 deletions src/darwin/Framework/CHIP/MTRAsyncWorkQueue_Internal.h

This file was deleted.

6 changes: 2 additions & 4 deletions src/darwin/Framework/CHIP/MTRDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#import <Matter/MTRDefines.h>
#import <os/lock.h>

#import "MTRAsyncWorkQueue_Internal.h"
#import "MTRAsyncWorkQueue.h"
#import "MTRAttributeSpecifiedCheck.h"
#import "MTRBaseDevice_Internal.h"
#import "MTRBaseSubscriptionCallback.h"
Expand Down Expand Up @@ -882,7 +882,7 @@ static BOOL AttributeHasChangesOmittedQuality(MTRAttributePath * attributePath)
NSArray * readRequestData = @[ readRequestPath, params ?: [NSNull null] ];

// But first, check if a duplicate read request is already queued and return
if ([_asyncWorkQueue isDuplicateForTypeID:MTRDeviceWorkItemDuplicateReadTypeID workItemData:readRequestData]) {
if ([_asyncWorkQueue hasDuplicateForTypeID:MTRDeviceWorkItemDuplicateReadTypeID workItemData:readRequestData]) {
return attributeValueToReturn;
}

Expand All @@ -894,8 +894,6 @@ static BOOL AttributeHasChangesOmittedQuality(MTRAttributePath * attributePath)
NSMutableArray<NSArray *> * readRequestsCurrent = opaqueDataCurrent;
NSMutableArray<NSArray *> * readRequestsNext = opaqueDataNext;

*fullyMerged = NO;

// Can only read up to 9 paths at a time, per spec
if (readRequestsCurrent.count >= 9) {
MTR_LOG_DEFAULT("%@ batching cannot add more", logPrefix);
Expand Down
Loading

0 comments on commit d4101be

Please sign in to comment.