CODEWITHSUNDEEP
cocoacore-datamemory-management
Kristian K
Kristian K

Reputation: 1

Is there a more memory efficient way to search through a Core Data database?

I need to see if an object that I have obtained from a CSV file with a unique identifier exists in my Core Data Database, and this is the code I deemed suitable for this task:

NSFetchRequest *fetchRequest = [[NSFetchRequest alloc] init];
NSEntityDescription *entity;
entity = 
[NSEntityDescription entityForName:@"ICD9"
            inManagedObjectContext:passedContext];
[fetchRequest setEntity:entity];
NSPredicate *pred = [NSPredicate predicateWithFormat:@"uniqueID like %@", uniqueIdentifier];
[fetchRequest setPredicate:pred];
NSError *err;
NSArray* icd9s = [passedContext executeFetchRequest:fetchRequest error:&err];
[fetchRequest release];
if ([icd9s count] > 0) {
    for (int i = 0; i < [icd9s count]; i++) {
        NSAutoreleasePool *pool = [[NSAutoreleasePool alloc]init];
        NSString *name = [[icd9s objectAtIndex:i] valueForKey:@"uniqueID"];
        if ([name caseInsensitiveCompare:uniqueIdentifier] == NSOrderedSame && name != nil)
        {
            [pool release];
            return [icd9s objectAtIndex:i];
        }
        [pool release];
    }
}
return nil;

After more thorough testing it appears that this code is responsible for a huge amount of leaking in the app I'm writing (it crashes on a 3GS before making it 20 percent through the 1459 items). I feel like this isn't the most efficient way to do this, any suggestions for a more memory efficient way? Thanks in advance!

Upvotes: 0

Views: 157

Answers (2)

Dave DeLong
Dave DeLong

Reputation: 243156

  • Don't use the like operator in your request predicate. Use =. That should be much faster.
  • You can specify the case insensitivity of the search via the predicate, using the [c] modifier.
  • It's not necessary to create and destroy an NSAutoreleasePool on each iteration of your loop. In fact, it's probably not needed at all.
  • You don't need to do any of the checking inside the for() loop. You're duplicating the work of your predicate.

So I would change your code to be:

NSFetchRequest *fetchRequest = [[NSFetchRequest alloc] init];
[fetchRequest setEntity:...];
[fetchRequest setPredicate:[NSPredicate predicateWithFormat:@"uniqueID =[c] %@", uniqueIdentifier]];
NSError *err = nil;
NSArray *icd9s = [passedContext executeFetchRequest:fetchRequest error:&err];
[fetchRequest release];
if (error == nil && [icd9s count] > 0) {
  return [icd9s objectAtIndex:0]; //we know the uniqueID matches, because of the predicate
}
return nil;

Upvotes: 2

Peter Hosey
Peter Hosey

Reputation: 96373

Use the Leaks template in Instruments to hunt down the leak(s). Your current code may be just fine once you fix them. The leak(s) may even be somewhere other than code.

Other problems:

  • Using fast enumeration will make the loop over the array (1) faster and (2) much easier to read.
  • Don't send release to an autorelease pool. If you ever port the code to garbage-collected Cocoa, the pool will not do anything. Instead, send it drain; in retain-release Cocoa and in Cocoa Touch, this works the same as release, and in garbage-collected Cocoa, it pokes the garbage collector, which is the closest equivalent in GC-land to draining the pool.
  • Don't repeat yourself. You currently have two [pool release]; lines for one pool, which gets every experienced Cocoa and Cocoa Touch programmer really worried. Store the result of your tests upon the name in a Boolean variable, then drain the pool before the condition, then conditionally return the object.
  • Be careful with variable types. -[NSArray count] returns and -[NSArray objectAtIndex:] takes an NSUInteger, not an int. Try to keep all your types matching at all times. (Switching to fast enumeration will, of course, solve this instance of this problem in a different way.)
  • Don't hide releases. I almost accused you of leaking the fetch request, then noticed that you'd buried it in the middle of the code. Make your releases prominent so that you're less likely to accidentally add redundant (i.e., crash-inducing) ones.

Upvotes: 1

Related Questions