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.

Sign up for GitHub

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

Jump to bottom

Progress reporting functionality #34

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Progress reporting functionality #34

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fd9e96a
Added interface for progress reporting to RXPromise. Changed implemen…
soxjke Mar 9, 2016
f7360ac
Implemented progress reporting & tests
soxjke Mar 11, 2016
0fc2a55
Chain of progress + tests
soxjke Mar 11, 2016
ecdf098
Fix data races in unit tests
Mar 17, 2016
e37599f
Merge pull request #1 from couchdeveloper/soxjke-master
soxjke Mar 20, 2016
1bb0ef8
Fixed review notes
soxjke Mar 20, 2016
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
6 changes: 0 additions & 6 deletions RXPromise Libraries/Source/RXPromise.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -634,12 +634,6 @@ typedef RXPromise* (^progress_on_main_block_t)(promise_progressHandler_t onProgr
*/
@property (nonatomic, readonly) BOOL isCancelled;

/*!
Returns the progress value of an underlying operation.
*/
@property (nonatomic, readonly) float progressValue;


/*!
Returns the parent promise - the promise which created
the receiver.
Expand Down
33 changes: 9 additions & 24 deletions RXPromise Libraries/Source/RXPromise.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,15 +146,13 @@ @implementation RXPromise {
id _result;
RXPromise_State _state;

std::list<std::pair<id,promise_progressHandler_t>> *_progressHandlers;
OSSpinLock __volatile _progressSpinLock;
std::list<RXPromise* __weak> *_children;
std::list<std::pair<id,promise_progressHandler_t>> _progressHandlers;
std::list<RXPromise* __weak> _children;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ivar _children can possibly be omitted. Please read my comments, and verify my assumptions.

}

@synthesize result = _result;
@synthesize parent = _parent;



- (void) dealloc {
DLogInfo(@"dealloc: %p", (__bridge void*)self);
if (_handler_queue) {
Expand All @@ -164,8 +162,6 @@ - (void) dealloc {
}
}
void const* key = (__bridge void const*)(self);
delete _progressHandlers;
delete _children;
if (dispatch_get_specific(rxpromise::shared::QueueID) == rxpromise::shared::sync_queue_id) {
Shared.assocs.erase(key);
} else {
Expand Down Expand Up @@ -357,10 +353,8 @@ - (void)synced_updateWithProgress:(float)progress {
if (_state != Pending) {
return;
}
OSSpinLockLock(&_progressSpinLock);
_progressValue = progress;
if (_progressHandlers) {
for (const std::pair<id, promise_progressHandler_t> handlerPair : *_progressHandlers) {
if (_progressHandlers.size()) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The enclosing if-statement if (_progressHandlers.size()) (not the block!) may be omitted. Just iterating over an empty list should work, too.

for (const std::pair<id, promise_progressHandler_t> handlerPair : _progressHandlers) {
id executionContext = handlerPair.first;
promise_progressHandler_t handlerBlock = handlerPair.second;
if (executionContext == Shared.default_concurrent_queue) {
Expand All @@ -380,12 +374,11 @@ - (void)synced_updateWithProgress:(float)progress {
}
}
}
if (_children) {
for(const RXPromise * __weak child : *_children) {
if (_children.size()) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The enclosing if-statment if (_children.size()) (not the block!) may be omitted. Just iterating over an empty list should work, too.

IFF _children can be replaced by Shared.assocs, it may look as follows:

void const* key = (__bridge void const*)(self);
auto range = Shared.assocs.equal_range(key);
while (range.first != range.second) {
    [(*(range.first)).second updateWithProgress:progress];
    ++range.first;
}

Then lines 447 till 456 can be deleted:

                             DLogInfo(@"%p add child %p", (__bridge void*)(blockSelf), (__bridge void*)(strongReturnedPromise));
                             if (executionContext == Shared.sync_queue) {
                                 // prerequiste: the block must have been enqueued with a barrier!
                                 Shared.assocs.emplace((__bridge void*)(blockSelf), weakReturnedPromise);
                             } else {
                                 void const* parent_pointer = (__bridge void*)(blockSelf);
                                 dispatch_barrier_async(Shared.sync_queue, ^{  // TODO:
                                     Shared.assocs.emplace(parent_pointer, weakReturnedPromise);
                                 }); 
                             }

Original source: https://github.com/couchdeveloper/RXPromise/blob/master/RXPromise%20Libraries/Source/RXPromise.mm#L402-L411

for(const RXPromise * __weak child : _children) {
[child updateWithProgress:progress];
}
}
OSSpinLockUnlock(&_progressSpinLock);
}


Expand All @@ -412,20 +405,12 @@ - (instancetype) registerWithExecutionContext:(id)executionContext
if (_handler_queue == nil) {
_handler_queue = createHandlerQueue(_state == Pending, (__bridge void*)self);
}
OSSpinLockLock(&_progressSpinLock);
if (onProgress) {
if (!_progressHandlers) {
_progressHandlers = new std::list<std::pair<id, promise_progressHandler_t>>();
}
_progressHandlers->push_back({executionContext, onProgress});
}
if (!_children) {
_children = new std::list<RXPromise* __weak>();
_progressHandlers.push_back({executionContext, onProgress});
}
if (weakReturnedPromise) {
_children->push_back(weakReturnedPromise);
_children.push_back(weakReturnedPromise);
}
OSSpinLockUnlock(&_progressSpinLock);

// Finally, *enqueue* a wrapper block which eventually gets invoked when the
// promise will be resolved:
Expand Down