Reputation: 11790
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.
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
Reputation: 22948
You should really use an NSOperationQueue
to handle execution of the NSOperation
s. 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
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