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 <r...@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 <r...@gnu.org> * 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 Gnustep-cvs@gna.org https://mail.gna.org/listinfo/gnustep-cvs