Skip to content

Commit

Permalink
fix for bug #47926
Browse files Browse the repository at this point in the history 
git-svn-id: svn+ssh://svn.gna.org/svn/gnustep/libs/base/trunk@40007 72102866-910b-0410-8b05-ffd578937521
  • Loading branch information
@rfm
rfm committed Jul 18, 2016
1 parent 9f0d813 commit 9188a05
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 22 deletions.
8 changes: 8 additions & 0 deletions ChangeLog
View file
@@ -1,3 +1,11 @@
2016-07-18 Richard Frith-Macdonald <rfm@gnu.org>

* Source/NSOperation.m: avoid sorting the queue ... keep the array of
waiting operations sorted by inserting new operations at the correct
position and observing the queuePriority of waiting operations (and
re-positiuoning them in the waiting array as necessary).
Fix for scalability problem (bug #47926)

2016-07-16 Richard Frith-Macdonald <rfm@gnu.org>

* Source/win32/GSRunLoopCtxt.m: fix bug in return value when polling.
Expand Down
59 changes: 41 additions & 18 deletions Source/NSOperation.m
View file
Expand Up @@ -64,12 +64,17 @@
#import "Foundation/NSException.h"
#import "Foundation/NSKeyValueObserving.h"
#import "Foundation/NSThread.h"
#import "GNUstepBase/NSArray+GNUstepBase.h"
#import "GSPrivate.h"

#define GSInternal NSOperationInternal
#include "GSInternal.h"
GS_PRIVATE_INTERNAL(NSOperation)

static void *isFinishedCtxt = (void*)"isFinished";
static void *isReadyCtxt = (void*)"isReady";
static void *queuePriorityCtxt = (void*)"queuePriority";

/* The pool of threads for 'non-concurrent' operations in a queue.
*/
#define POOL 8
Expand Down Expand Up @@ -135,7 +140,7 @@ - (void) addDependency: (NSOperation *)op
[op addObserver: self
forKeyPath: @"isFinished"
options: NSKeyValueObservingOptionNew
context: NULL];
context: isFinishedCtxt];
if (internal->ready == YES)
{
/* The new dependency stops us being ready ...
Expand Down Expand Up @@ -245,7 +250,7 @@ - (id) init
[self addObserver: self
forKeyPath: @"isFinished"
options: NSKeyValueObservingOptionNew
context: NULL];
context: isFinishedCtxt];
}
return self;
}
Expand Down Expand Up @@ -355,7 +360,7 @@ - (void) removeDependency: (NSOperation *)op
[self observeValueForKeyPath: @"isFinished"
ofObject: op
change: nil
context: nil];
context: isFinishedCtxt];
}
[self didChangeValueForKey: @"dependencies"];
}
Expand Down Expand Up @@ -603,7 +608,7 @@ - (void) addOperation: (NSOperation *)op
[op addObserver: self
forKeyPath: @"isReady"
options: NSKeyValueObservingOptionNew
context: NULL];
context: isReadyCtxt];
[self willChangeValueForKey: @"operations"];
[self willChangeValueForKey: @"operationCount"];
[internal->operations addObject: op];
Expand All @@ -614,7 +619,7 @@ - (void) addOperation: (NSOperation *)op
[self observeValueForKeyPath: @"isReady"
ofObject: op
change: nil
context: nil];
context: isReadyCtxt];
}
}
[internal->lock unlock];
Expand Down Expand Up @@ -679,7 +684,7 @@ - (void) addOperations: (NSArray *)ops
[op addObserver: self
forKeyPath: @"isReady"
options: NSKeyValueObservingOptionNew
context: NULL];
context: isReadyCtxt];
[internal->operations addObject: op];
if (NO == [op isReady])
{
Expand All @@ -697,7 +702,7 @@ - (void) addOperations: (NSArray *)ops
[self observeValueForKeyPath: @"isReady"
ofObject: op
change: nil
context: nil];
context: isReadyCtxt];
}
}
[internal->lock unlock];
Expand Down Expand Up @@ -867,9 +872,14 @@ - (void) observeValueForKeyPath: (NSString *)keyPath
change: (NSDictionary *)change
context: (void *)context
{
[internal->lock lock];
if (YES == [object isFinished])
/* We observe three properties in sequence ...
* isReady (while we wait for an operation to be ready)
* queuePriority (when priority of a ready operation may change)
* isFinished (to see if an executing operation is over).
*/
if (context == isFinishedCtxt)
{
[internal->lock lock];
internal->executing--;
[object removeObserver: self
forKeyPath: @"isFinished"];
Expand All @@ -882,11 +892,27 @@ - (void) observeValueForKeyPath: (NSString *)keyPath
[self didChangeValueForKey: @"operationCount"];
[self didChangeValueForKey: @"operations"];
}
else if (YES == [object isReady])
else if (context == queuePriorityCtxt || context == isReadyCtxt)
{
[object removeObserver: self
forKeyPath: @"isReady"];
[internal->waiting addObject: object];
NSInteger pos;

[internal->lock lock];
if (context == queuePriorityCtxt)
{
[internal->waiting removeObjectIdenticalTo: object];
}
if (context == isReadyCtxt)
{
[object removeObserver: self forKeyPath: @"isReady"];
[object addObserver: self
forKeyPath: @"queuePriority"
options: NSKeyValueObservingOptionNew
context: queuePriorityCtxt];
}
pos = [internal->waiting insertionPosition: object
usingFunction: sortFunc
context: 0];
[internal->waiting insertObject: object atIndex: pos];
[internal->lock unlock];
}
[self _execute];
Expand Down Expand Up @@ -985,21 +1011,18 @@ - (void) _execute
{
NSOperation *op;

/* Make sure we have a sorted queue of operations waiting to execute.
*/
[internal->waiting sortUsingFunction: sortFunc context: 0];

/* Take the first operation from the queue and start it executing.
* We set ourselves up as an observer for the operating finishing
* and we keep track of the count of operations we have started,
* but the actual startup is left to the NSOperation -start method.
*/
op = [internal->waiting objectAtIndex: 0];
[internal->waiting removeObjectAtIndex: 0];
[op removeObserver: self forKeyPath: @"queuePriority"];
[op addObserver: self
forKeyPath: @"isFinished"
options: NSKeyValueObservingOptionNew
context: NULL];
context: isFinishedCtxt];
internal->executing++;
if (YES == [op isConcurrent])
{
Expand Down
36 changes: 32 additions & 4 deletions Tests/base/NSOperation/threads.m
View file
Expand Up @@ -83,13 +83,13 @@ - (NSThread*) thread

- (void) release
{
NSLog(@"Will release %@ at %@", self, [NSThread callStackSymbols]);
// NSLog(@"Will release %@ at %@", self, [NSThread callStackSymbols]);
[super release];
}

- (id) retain
{
NSLog(@"Will retain %@ at %@", self, [NSThread callStackSymbols]);
// NSLog(@"Will retain %@ at %@", self, [NSThread callStackSymbols]);
return [super retain];
}

Expand Down Expand Up @@ -218,7 +218,8 @@ int main()
[q addOperation: obj];
[q waitUntilAllOperationsAreFinished];
PASS(([obj isFinished] == YES), "main queue runs an operation");
PASS(([obj thread] != [NSThread currentThread]), "operation ran in other thread");
PASS(([obj thread] != [NSThread currentThread]),
"operation ran in other thread");

[q setSuspended: YES];
obj = [OpFlag new];
Expand Down Expand Up @@ -256,6 +257,31 @@ int main()
[q addOperations: a waitUntilFinished: YES];
PASS(([list isEqual: a]), "operations ran in order of addition");

[list removeAllObjects];
[a removeAllObjects];
[q setSuspended: YES];
obj = [OpOrder new];
[obj setQueuePriority: NSOperationQueuePriorityHigh];
[a addObject: obj];
[q addOperation: obj];
[obj release];
obj = [OpOrder new];
[a addObject: obj];
[q addOperation: obj];
[obj release];
obj = [OpOrder new];
[obj setQueuePriority: NSOperationQueuePriorityLow];
[a addObject: obj];
[q addOperation: obj];
[obj release];
obj = [a objectAtIndex: 1];
[obj setQueuePriority: NSOperationQueuePriorityVeryLow];
[a addObject: obj];
[a removeObjectAtIndex: 1];
[q setSuspended: NO];
[q waitUntilAllOperationsAreFinished];
PASS(([list isEqual: a]), "operations ran in order of priority");

[list removeAllObjects];
[a removeAllObjects];
[q setSuspended: YES];
Expand All @@ -274,7 +300,9 @@ int main()
[obj addDependency: old];
[q setSuspended: NO];
[q addOperations: a waitUntilFinished: YES];
PASS(([list objectAtIndex: 0] == [a objectAtIndex: 1] && [list objectAtIndex: 1] == [a objectAtIndex: 0]), "operations ran in order of dependency");
PASS(([list objectAtIndex: 0] == [a objectAtIndex: 1]
&& [list objectAtIndex: 1] == [a objectAtIndex: 0]),
"operations ran in order of dependency");
PASS(1 == [[old dependencies] count], "dependencies not removed when done")

[arp release]; arp = nil;
Expand Down

0 comments on commit 9188a05

Please sign in to comment.