Vikas Bansal
Vikas Bansal

Reputation: 11790

COCOA: Unable to click CANCEL button, UI gets hang and unresponsive, how to deal with it?

Please let me elaborate what I am doing and what's happening. I am new to cocoa and objective-c. Please be a little lenient. thank you.

What I am doing

I am creating a file search utility where user will enter a keyword and the cocoa application will search the whole system files and if the keyword is present in the file path then it show it to the user.

POC: http://1drv.ms/23J0WJQ

Problem

When clicking on CANCEL button the UI gets hand and become unresponsive.

Coding part starts here:

library

/// this funciton will be called with
/// keyword : the word that should be present in the file path
/// selector : a function tha will be called when ever the function will find a matching file
/// selector: object of the class that has the funciton (selector)
-(NSMutableArray *)searchWholeSystem:(NSString *)keyword
                            selector:(SEL)aselector
                              target:(id)atarget
{
    /// _cancelSearch is a property which can be set by the user of the class
    /// when the scann will be started it will be set to as default : NO
    _cancelSearch = NO;

    /// list of all the files that are matched with the keyword
    NSMutableArray* fileList = [[NSMutableArray alloc]init];

    ///autoreleasepool to release the unused memory
    @autoreleasepool {

        ///getMainDirectories is local function that will get the user direcotires or the target direcoties where the search will be done
        NSMutableArray* directories = [self getMainDirectories];

        ///one by one search all the direcoties for the matching files
        for (NSString* dir in directories) {

            NSMutableArray* resultList = [self getAllFiles:dir tag:keyword target:atarget selector:aselector];

            for (int i = 0; i<[resultList count]; i++) {

                ///if cancel then return the as yet result array
                if (_cancelSearch)
                    return fileList;

                NSString* path = [resultList objectAtIndex:i];

                GenericResultModel* sfile = [[GenericResultModel alloc]init];

                sfile.Column1 = [path lastPathComponent];
                sfile.Column2 = path;


                [fileList addObject:sfile];

            }
        }



        return fileList;
    }
}

///getAllFiles will be having
///sPath : source path where the search will be performed
///tag: tag is the keyword that need to found in the path
/// selector : a function tha will be called when ever the function will find a matching file
/// selector: object of the class that has the funciton (selector)
-(NSMutableArray*)getAllFiles:(NSString*)sPath tag:(NSString*)_tag target:(id)atarget selector:(SEL)aselector
{

    //  fileList is the result that will contain all the file names that has the _tag
    NSMutableArray* fileList = [[NSMutableArray alloc]init];


    @autoreleasepool {

        //   _tag is the keyword that should be present in the file name
        _tag = [_tag lowercaseString];



        ///getting all contents of the source path
        NSArray *contentOfDirectory=[[NSFileManager defaultManager] contentsOfDirectoryAtPath:sPath error:NULL];

        for(int i=0; i < [contentOfDirectory count]; i++)
        {
            if (_cancelSearch)
                return fileList;

            NSString *filePathInDirectory = [contentOfDirectory objectAtIndex:i];

            NSString* fullPath = [NSString stringWithFormat:@"%@/%@", sPath, filePathInDirectory];

            if ([FMH isdirOrApp:fullPath])
            {
                if ([[fullPath lowercaseString] rangeOfString:_tag].location != NSNotFound && [fileList indexOfObject:fullPath] == NSNotFound)
                {
                    [fileList addObject:fullPath];
                    [atarget performSelector:aselector withObject:fullPath];
                }

                if (_cancelSearch)
                    return fileList;


                NSMutableArray* files =  [self getAllFiles:fullPath tag:_tag target:atarget selector:aselector];

                for (NSString* f in files)
                {
                    if ([[f lowercaseString] rangeOfString:_tag].location != NSNotFound && [fileList indexOfObject:fullPath] == NSNotFound)
                    {
                        [fileList addObject:f];
                        [atarget performSelector:aselector withObject:f];
                    }

                    if (_cancelSearch)
                        return fileList;
                }
            }
            else
            {
                NSString* fileN = [fullPath lastPathComponent];
                if ([[fileN lowercaseString] rangeOfString:_tag].location != NSNotFound && [fileList indexOfObject:fullPath] == NSNotFound)
                {
                    [fileList addObject:fullPath];
                    [atarget performSelector:aselector withObject:fullPath];
                }

                if (_cancelSearch)
                    return fileList;
            }
        }

    }

    return fileList;

}

calling the library function from the UI

-(void)cancelClick:(id)sender
{
    macSearch.cancelSearch = YES; ///setting the cancel to yes so that the library function may come to know that the search is canceled and now stop searching return what ever is searched
}

-(void)wholeSystemScan:(id)sender
{

    if([[searchKeyword stringValue] length] < 1)
    {
        [[[MessageBoxHelper alloc] init] ShowMessageBox:@"Please enter a keyword to search." Title:@"Information" IsAlert:YES];

        return;
    }

    [self ScanStartDisableControls];

    NSString* keyw = [searchKeyword stringValue];


    dispatch_queue_t backgroundQueue = dispatch_queue_create("com.techheal.fileSearch", 0);

    dispatch_async(backgroundQueue, ^{

        [macSearch searchWholeSystem:keyw selector:@selector(refreshSelector:)  target:self];

        [self ScanCompletedEnableControls];

    });

}

Upvotes: 3

Views: 74

Answers (2)

NSGod
NSGod

Reputation: 22948

You should really use an NSOperationQueue to handle execution of the NSOperations. When you call -start on the operation yourself, the operation is taking place on the thread in which -start is being called, namely, the main thread. This is probably not what you want, since by doing all of the work on the main thread, the attempts to update the UI won't be able to come through until after the operation finishes. To fix that problem, just use an NSOperationQueue.

You create the NSOperationQueue (queue) in the init method. Then your start: method looks like this:

-(void)start:(id)sender
{
    [DataList removeAllObjects];
    [tableView reloadData];

    NSString* keyw = [searchTextBox stringValue];

    searcher = [[MacFileSearchReVamp alloc] initWithFileName:keyw selector:@selector(refreshSelector:)  target:self];

    searcher.delegate = self;

    [queue addOperation:searcher];
//    [searcher startSearch];
}

You can see here that instead of calling startSearch directly, we simply add the searcher object to the queue, and it handles executing the operation on a background thread.

Your stop: method becomes:

- (IBAction)stop:(id)sender
{
    [queue cancelAllOperations];
//    [searcher stopSearch];
}

Then, to address performance issues that freeze the UI when you have a large number of search results. What's happening in your current code is that the background thread is finding results so fast and trying to call the main thread to update with each result that the main thread gets overloaded with work so becomes unresponsive. To alleviate this, you need to make much fewer calls to the main thread to update the UI. While there are numerous ways to do this, one way is to simply have the background thread store up its results for 0.5 seconds, then call the main thread and pass those results. It then repeats this every 0.5 seconds until it's finished. While not perfect, it should improve the responsiveness.

Also, while the following changes may not be needed, to me they seem like a clearer design. When you want to communicate from an NSOperation object that's being run in a background thread, to do something like update the UI which should be done on the main thread, have the operation object itself worry about being sure to call the refresh selector on the main thread. So, remove the dispatch_async call, and also change the refresh selector to be a method that accepts an array of paths:

-(void)refreshSelectorWithPaths:(NSArray *)resultPaths
{
    for (NSString *resultPath in resultPaths) {
        GenericResultModel* sfile = [[GenericResultModel alloc]init];
        sfile.Column1 = [resultPath lastPathComponent];
        sfile.Column2 = resultPath;
        [DataList addObject:sfile];
    }
    [tableView reloadData];
}

You should remove the code that checks to see if the DataList already contains the entry, as it will be disastrous for performance as the number of results increase, and, given the updated NSOperation code, it will be unnecessary.

#define MD_PROGRESS_UPDATE_TIME_INTERVAL 0.5

-(NSMutableArray*)getAllFiles:(NSString*)sPath tag:(NSString*)_tag target:(id)atarget selector:(SEL)aselector {
    //  fileList is the result that will contain all the file names that has the _tag
    NSMutableArray* fileList = [[NSMutableArray alloc]init];
    NSMutableArray *fullPaths = [NSMutableArray array];

    @autoreleasepool {
        //   _tag is the keyword that should be present in the file name
        _tag = [_tag lowercaseString];

 /* subpathsOfDirectoryAtPath:error: gets all subpaths recursively 
  eliminating need for calling this method recursively, and eliminates
   duplicate results */
        NSArray *subpaths = [[NSFileManager defaultManager] subpathsOfDirectoryAtPath:sPath error:NULL];

        NSUInteger subpathsCount = subpaths.count;
        NSDate *progressDate = [NSDate date];

        for (NSUInteger i = 0; i < subpathsCount; i++) {
            if ([self isCancelled]) break;

            NSString *subpath = [subpaths objectAtIndex:i];
            if ([[[subpath lastPathComponent] lowercaseString] rangeOfString:_tag].location != NSNotFound) {
                NSString *fullPath = [sPath stringByAppendingPathComponent:subpath];
                [fileList addObject:fullPath];
                [fullPaths addObject:fullPath];
                if (ABS([progressDate timeIntervalSinceNow]) > MD_PROGRESS_UPDATE_TIME_INTERVAL) {
                    [atarget performSelectorOnMainThread:aselector withObject:fullPaths waitUntilDone:NO];
                    [fullPaths removeAllObjects];
                    progressDate = [NSDate date];
                }
            }
        }
        if (fullPaths.count) [atarget performSelectorOnMainThread:aselector withObject:fullPaths waitUntilDone:NO];
    }
    return fileList;
}

The above code uses fullPaths to store the full paths for each 0.5 second interval. As a result is found, path is added to fullPaths, and then we check if it's been 0.5 seconds yet since the last time we told the main thread to refresh. If it has, we call the refresh selector and then remove those entries from the fullPaths array.

Here's a revamped version of your Proof-of-concept (updated with performance enhancements):

http://www.markdouma.com/developer/mySearch.zip

Upvotes: 2

aman.sood
aman.sood

Reputation: 874

Library "SearchFile.h" file

@protocol SearchFileDelegate <NSObject>

- (void)completionWithSearchList:(NSArray*)serachList;

@end

@interface SearchFile : NSOperation
@property(nonatomic, weak)id<SearchFileDelegate> delegate;
-(id)initWithFileName:(NSString*)fileName;
-(void)startSearch;
-(void)stopSearch;

@end

"SearchFile.m" file

#import "SearchFile.h"

@interface SearchFile ()
@property(nonatomic, strong)NSMutableArray* searchList;
@property(nonatomic, strong)NSString* fileName;

@end

@implementation SearchFile

-(id)initWithFileName:(NSString*)fileName
{
  self = [super init];
  self.fileName = fileName;
  return self;
}

- (void)main
{
  // add logic to search file.
  for (<#initialization#>; <#condition#>; <#increment#>) {
    // check for operation being canceled
      if([self isCancelled])
      {
        break;
      }
  }
    /
  // finally if search is complete or canceled
  [self.delegate completionWithSearchList:self.searchList];
}

-(void)startSearch
{
  [self start];
}

-(void)stopSearch
{
  [self cancel];
}

NSOperation interface will handle all your requirements. Your calling interface needs to set delegate property of SearchFile and implement - -(void)completionWithSearchList:(NSArray*)serachList, call

-(void)startSearch;
-(void)stopSearch

when required . You can go ahead and modify as per your requirement to handle any error conditions as well

Upvotes: 1

Related Questions