5
\$\begingroup\$

I have some efficient methods that recursively creates a tree structure (NSTreeNode) of by looping recursively over a directory structure. The result is displayed in a NSOutlineView. This works well with one exception where I have to non-concurrently update a NSMutableDictionary that holds the structure of the tree. This turns out to be a real bottleneck in my program.

I am wondering if anyone has suggestions for how I can make this method more efficient or avoid the whole method.

-(void) addDictionaryItem:(NSString *) mykey withURL:(NSString *)myurl isDir:(BOOL) myIsDir andLR:(NSString *)LR
{
    NSDictionary *mydict=nil;

    @synchronized(@"concurrentDictAccess") {
        mydict = [self.dict objectForKey:mykey];
    }
    if ( mydict !=nil) {

            NSMutableArray *myarray = [mydict objectForKey:@"myarray"];
            NSMutableArray *myarrayIsDir = [mydict objectForKey:@"isdir"];
            NSMutableArray *myarrayLR = [mydict objectForKey:@"LR"];

            if (![myarray containsObject:myurl]){
                [myarray addObject:myurl];
                [myarrayLR addObject:LR];
                [myarrayIsDir addObject:[NSNumber numberWithBool:myIsDir]];
            } 
        }
    else {

        NSMutableArray *arrayOfFiles = [NSMutableArray array];
        [arrayOfFiles addObject:myurl];

        NSMutableArray *arrayIsDir = [NSMutableArray array];
        [arrayIsDir addObject:[NSNumber numberWithBool:myIsDir]];

        NSMutableArray *arrayOfLR = [NSMutableArray array];
        [arrayOfLR addObject:LR];

        NSMutableDictionary *attrDict = [NSMutableDictionary   dictionaryWithObjectsAndKeys:arrayOfLR, @"LR",
                                     arrayOfFiles, @"myarray", arrayIsDir, @"isdir",
                                     nil];
        @synchronized(@"concurrentDictAccess") {
             [self.dict setObject:attrDict forKey:mykey];
        }
    }
}
\$\endgroup\$
6
\$\begingroup\$

The performance might be improved if you used more efficient synchronization mechanism. The @synchronized directive is notoriously inefficient. It's pretty convenient way of doing mutex lock, but there are more efficient alternatives.

A common pattern is to use dedicated GCD serial queue to synchronize access. Even better, one can use the the reader-writer pattern: It's a concurrent GCD queue where you dispatch_sync to read operations to enjoy concurrency on read operations, but use dispatch_barrier_async to write). See the discussion of this pattern in WWDC 2012 video Asynchronous Design Patterns with Blocks, GCD, and XPC (it's in the latter portion of the video).

A couple of other reactions to the provided code snippet:

  1. If you use @synchronized, I wouldn't suggest using string constants like that. The typical convention is to use the object that is being mutated (thus make sure it is instantiated up-front, and then you can do @synchronized (self.dict)).

  2. Note, the provided code is not thread-safe. It has several issues:

    • One cannot just synchronize the initial read and final write separately. You generally have to wrap the whole routine in some synchronization mechanism. (And obviously, make sure any reads outside of this method are synchronized, too.)

    • BTW, the current code that adds of the entries to the existing array isn't currently synchronized at all.

  3. This is really unrelated, but I wouldn't suggest three separate arrays for the URL, LR (?), and isdir. I'd personally rather see an array of objects (either custom object or NSDictionary for which each object has its own URL, LR, and isDir). There are other approaches, too, but having three separate arrays doesn't seem very appealing.

  4. Also unrelated, but I'm surprised that if you call this method and the key/URL both exist, that you don't update the existing record. Right now, you leave the old values there.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.