Author: rfm
Date: Mon Jul 18 11:51:35 2016
New Revision: 40007
URL: http://svn.gna.org/viewcvs/gnustep?rev=40007&view=rev
Log:
fix for bug #47926
Modified:
libs/base/trunk/ChangeLog
libs/base/trunk/Source/NSOperation.m
libs/base/trunk/Tests/base/NSOperation/threads.m
Modified: libs/base/trunk/ChangeLog
URL:
http://svn.gna.org/viewcvs/gnustep/libs/base/trunk/ChangeLog?rev=40007&r1=40006&r2=40007&view=diff
==============================================================================
--- libs/base/trunk/ChangeLog (original)
+++ libs/base/trunk/ChangeLog Mon Jul 18 11:51:35 2016
@@ -1,3 +1,11 @@
+2016-07-18 Richard Frith-Macdonald <[email protected]>
+
+ * 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 <[email protected]>
* Source/win32/GSRunLoopCtxt.m: fix bug in return value when polling.
Modified: libs/base/trunk/Source/NSOperation.m
URL:
http://svn.gna.org/viewcvs/gnustep/libs/base/trunk/Source/NSOperation.m?rev=40007&r1=40006&r2=40007&view=diff
==============================================================================
--- libs/base/trunk/Source/NSOperation.m (original)
+++ libs/base/trunk/Source/NSOperation.m Mon Jul 18 11:51:35 2016
@@ -64,11 +64,16 @@
#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.
*/
@@ -135,7 +140,7 @@
[op addObserver: self
forKeyPath: @"isFinished"
options: NSKeyValueObservingOptionNew
- context: NULL];
+ context: isFinishedCtxt];
if (internal->ready == YES)
{
/* The new dependency stops us being ready ...
@@ -245,7 +250,7 @@
[self addObserver: self
forKeyPath: @"isFinished"
options: NSKeyValueObservingOptionNew
- context: NULL];
+ context: isFinishedCtxt];
}
return self;
}
@@ -355,7 +360,7 @@
[self observeValueForKeyPath: @"isFinished"
ofObject: op
change: nil
- context: nil];
+ context: isFinishedCtxt];
}
[self didChangeValueForKey: @"dependencies"];
}
@@ -603,7 +608,7 @@
[op addObserver: self
forKeyPath: @"isReady"
options: NSKeyValueObservingOptionNew
- context: NULL];
+ context: isReadyCtxt];
[self willChangeValueForKey: @"operations"];
[self willChangeValueForKey: @"operationCount"];
[internal->operations addObject: op];
@@ -614,7 +619,7 @@
[self observeValueForKeyPath: @"isReady"
ofObject: op
change: nil
- context: nil];
+ context: isReadyCtxt];
}
}
[internal->lock unlock];
@@ -679,7 +684,7 @@
[op addObserver: self
forKeyPath: @"isReady"
options: NSKeyValueObservingOptionNew
- context: NULL];
+ context: isReadyCtxt];
[internal->operations addObject: op];
if (NO == [op isReady])
{
@@ -697,7 +702,7 @@
[self observeValueForKeyPath: @"isReady"
ofObject: op
change: nil
- context: nil];
+ context: isReadyCtxt];
}
}
[internal->lock unlock];
@@ -867,9 +872,14 @@
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"];
@@ -882,11 +892,27 @@
[self didChangeValueForKey: @"operationCount"];
[self didChangeValueForKey: @"operations"];
}
- else if (YES == [object isReady])
- {
- [object removeObserver: self
- forKeyPath: @"isReady"];
- [internal->waiting addObject: object];
+ else if (context == queuePriorityCtxt || context == isReadyCtxt)
+ {
+ 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];
@@ -984,10 +1010,6 @@
&& [internal->waiting count] > 0)
{
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
@@ -996,10 +1018,11 @@
*/
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])
{
Modified: libs/base/trunk/Tests/base/NSOperation/threads.m
URL:
http://svn.gna.org/viewcvs/gnustep/libs/base/trunk/Tests/base/NSOperation/threads.m?rev=40007&r1=40006&r2=40007&view=diff
==============================================================================
--- libs/base/trunk/Tests/base/NSOperation/threads.m (original)
+++ libs/base/trunk/Tests/base/NSOperation/threads.m Mon Jul 18 11:51:35 2016
@@ -83,13 +83,13 @@
- (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];
}
@@ -218,7 +218,8 @@
[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];
@@ -255,6 +256,31 @@
[q setSuspended: NO];
[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];
@@ -274,7 +300,9 @@
[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;
_______________________________________________
Gnustep-cvs mailing list
[email protected]
https://mail.gna.org/listinfo/gnustep-cvs
