Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Darwin] MTRDevice should coalesce reads and avoid duplicates #26999

Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/darwin/Framework/CHIP/MTRAsyncCallbackWorkQueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ typedef void (^MTRAsyncCallbackReadyHandler)(id context, NSUInteger retryCount);
// Work items may be enqueued from any queue or thread
// Note: Once a work item is enqueued, its handlers cannot be modified
- (void)enqueueWorkItem:(MTRAsyncCallbackQueueWorkItem *)item;

// TODO: Add a "set concurrency width" method to allow for more than 1 work item at a time
@end

// An item in the work queue
Expand Down
63 changes: 62 additions & 1 deletion src/darwin/Framework/CHIP/MTRAsyncCallbackWorkQueue.mm
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#import <dispatch/dispatch.h>
#import <os/lock.h>

#import "MTRAsyncCallbackWorkQueue.h"
#import "MTRAsyncCallbackWorkQueue_Internal.h"
#import "MTRLogging_Internal.h"

#pragma mark - Class extensions
Expand Down Expand Up @@ -169,9 +169,50 @@ - (void)_callNextReadyWorkItem
self.runningWorkItemCount = 1;

MTRAsyncCallbackQueueWorkItem * workItem = self.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)) {
while (self.items.count >= 2) {
MTRAsyncCallbackQueueWorkItem * nextWorkItem = self.items[1];
if (!nextWorkItem.batchable || (nextWorkItem.batchingID != workItem.batchingID)) {
// next item is not eligible to merge with this one
break;
}

BOOL fullyMerged = NO;
workItem.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;
}

[self.items removeObjectAtIndex:1];
}
}

[workItem callReadyHandlerWithContext:self.context];
}
}

- (BOOL)isDuplicateForTypeID:(NSUInteger)opaqueDuplicateTypeID workItemData:(id)opaqueWorkItemData
{
os_unfair_lock_lock(&_lock);
// Start from the last item
for (NSUInteger i = self.items.count; i > 0; i--) {
MTRAsyncCallbackQueueWorkItem * item = self.items[i - 1];
BOOL isDuplicate = NO;
BOOL stop = NO;
if (item.supportsDuplicateCheck && (item.duplicateTypeID == opaqueDuplicateTypeID) && item.duplicateCheckHandler) {
item.duplicateCheckHandler(opaqueWorkItemData, &isDuplicate, &stop);
if (stop) {
os_unfair_lock_unlock(&_lock);
return isDuplicate;
}
}
}
os_unfair_lock_unlock(&_lock);
return NO;
}
@end

@implementation MTRAsyncCallbackQueueWorkItem
Expand Down Expand Up @@ -277,4 +318,24 @@ - (void)cancel
});
}
}

- (void)setBatchingID:(NSUInteger)opaqueBatchingID
data:(id)opaqueBatchableData
handler:(MTRAsyncCallbackBatchingHandler)batchingHandler
{
os_unfair_lock_lock(&self->_lock);
_batchable = YES;
_batchingID = opaqueBatchingID;
_batchableData = opaqueBatchableData;
_batchingHandler = batchingHandler;
os_unfair_lock_unlock(&self->_lock);
}

- (void)setDuplicateTypeID:(NSUInteger)opaqueDuplicateTypeID handler:(MTRAsyncCallbackDuplicateCheckHandler)duplicateCheckHandler
{
_supportsDuplicateCheck = YES;
_duplicateTypeID = opaqueDuplicateTypeID;
_duplicateCheckHandler = duplicateCheckHandler;
}

@end
70 changes: 70 additions & 0 deletions src/darwin/Framework/CHIP/MTRAsyncCallbackWorkQueue_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,79 @@ NS_ASSUME_NONNULL_BEGIN

@class MTRDevice;

// Optional feature: Work Item Batching
// 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 client of this API an opportunity to coalesce and merge work items.
// - The "batching ID" is used for grouping mergeable work items with unique merging strategies. The ID value is opaque to this
// API, and the API client is responsible for assigning them.
// - Each work item will only be asked to batch before it's first dequeued to run readyHandler.
// See the MTRAsyncCallbackBatchingHandler definition for more details.

// Optional feature: Duplicate Filtering
jtung-apple marked this conversation as resolved.
Show resolved Hide resolved
// This is a facility that enables the API client to check if a potential work item has already been enqueued. By providing a
// handler that can answer if a work item's relevant data is a duplicate, it can avoid redundant queuing of requests.
// - The "duplicate type ID" is used for grouping different types of work items for duplicate checking. The ID value is opaque
// to this API, and the API client is responsible for assigning them.
// See the MTRAsyncCallbackDuplicateCheckHandler definition and the WorkQueue's -isDuplicateForTypeID:workItemData: and
// removeDuplicateCheckHandlerForTypeID:workItemData: method descriptions for more details.

// 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 dequeued and executed for the first time.
// 2) The next work item in the queue is also batchable.
// 3) The two work items have matching 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. Otherwise the handler should set *fullyMerged to NO.
//
// If *fullyMerged is set to YES, this handler may be called again to possibly also batch the work item
// after the one that was dropped.
typedef void (^MTRAsyncCallbackBatchingHandler)(id opaqueDataCurrent, id opaqueDataNext, BOOL * fullyMerged);

// The duplicate check handler is called by the work queue when the client wishes to check whether a work item is a duplicate of an
// existing one, so that the client may decide to not enqueue a duplicate work item.
//
// The handler will be passed the opaque data of a potential duplicate work item.
//
// If the handler determins the data is indeed duplicate work, it should set *stop to YES, and set *isDuplicate to YES.
jtung-apple marked this conversation as resolved.
Show resolved Hide resolved
//
// If the handler determins the data is not duplicate work, it should set *stop to YES, and set *isDuplicate to NO.
jtung-apple marked this conversation as resolved.
Show resolved Hide resolved
//
// If the handler is unable to determins if the data is duplicate work, it should set *stop to NO.
jtung-apple marked this conversation as resolved.
Show resolved Hide resolved
typedef void (^MTRAsyncCallbackDuplicateCheckHandler)(id opaqueItemData, BOOL * isDuplicate, BOOL * stop);

@interface MTRAsyncCallbackWorkQueue ()
// The MTRDevice object is only held and passed back as a reference and is opaque to the queue
- (instancetype)initWithContext:(id _Nullable)context queue:(dispatch_queue_t)queue;

// Before creating a work item, a client may call this method to check with existing work items that the new potential work item
// data is not a duplicate request.
// - 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, and if a handler sets *stop to YES, this method will return the value the handler sets for *isDuplicate
// - If no duplicate check handlers sets *stop to YES, this method will return NO.
jtung-apple marked this conversation as resolved.
Show resolved Hide resolved
- (BOOL)isDuplicateForTypeID:(NSUInteger)opaqueDuplicateTypeID workItemData:(id)opaqueWorkItemData;
@end

@interface MTRAsyncCallbackQueueWorkItem ()
// Batching
@property (nonatomic, readonly) BOOL batchable;
@property (nonatomic, readonly) NSUInteger batchingID;
@property (nonatomic, readonly) id batchableData;
@property (nonatomic, readonly) MTRAsyncCallbackBatchingHandler batchingHandler;
- (void)setBatchingID:(NSUInteger)opaqueBatchingID
data:(id)opaqueBatchableData
handler:(MTRAsyncCallbackBatchingHandler)batchingHandler;

// Duplicate check
@property (nonatomic, readonly) BOOL supportsDuplicateCheck;
@property (nonatomic, readonly) NSUInteger duplicateTypeID;
@property (nonatomic, readonly) MTRAsyncCallbackDuplicateCheckHandler duplicateCheckHandler;
- (void)setDuplicateTypeID:(NSUInteger)opaqueDuplicateTypeID handler:(MTRAsyncCallbackDuplicateCheckHandler)duplicateCheckHandler;
@end

NS_ASSUME_NONNULL_END
Loading