MR_create slow ! #386

Closed
ClarisseL opened this Issue Jan 15, 2013 · 8 comments

Comments

Projects
None yet
3 participants

Hi,
I really love your work but since I updated to 2.1 the MR_create takes waaayy much longer than it used to do, is it just me or known ? Or maybe I'm doing something wrong ?

I'm find-or-creating 3000 new objects with MR_create it used to take 5 sec on iPhone 5, and now it takes 35 ! I'm using saveWithBlock now.

Thank you very much !

EDIT : OK as I'm a newbie I didn't know how to make any test to really find out what was slow, and with a bit of luck I found that what was slowing was [NSManagedObject MR_findFirstByAttribute:attribute value inContext:localContext];
When I removed the inContext at the end the time went down from 35 to 3.5 sec ! I still can't understand why :/ But it's fixed !

PS : If you have a little time to kill I'd love to know the explanation ! :p

Member

blackgold9 commented Jan 15, 2013

Can you post your save/creation loop? That would really help us get to the bottom of it.

Sure, here

[MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
        for (NSDictionary *dict in dictionary) {
            if (_isCancelled == NO) { //This is part of an NSOperation
                NSString *url = [dict valueForKey:@"Link"]; // This is an url
                NSNumber *hash = [NSNumber numberWithInteger:url.hash]; //I Hash the url to get something unique I think it's more performant than using a complete url to search
                // This is the method where I removed "inContext:localContext"
                CustomObject *mds = [CustomObject MR_findFirstByAttribute:@"objectHash" withValue:hash];
                if (mds) {
                    //DDLogVerbose(@"Object exists already : %@", mds.title);
                }
                else {
                    //DDLogVerbose(@"Creating Object ...");
                    mds = [CustomObject MR_createInContext:localContext];
                    mds.objectHash = hash;
                    mds.title = [dict valueForKey:@"Title"];
                    mds.catalog = [NSNumber numberWithInteger:self.catalog];
                    mds.link = url;
                }
            }
        }
    } completion:^(BOOL success, NSError *error) {
        if (success) {
              //Not relevant stuff
        }
        else {
            DDLogError(@"-- AN ERROR OCCURED WHILE SAVING :%@ --", error);
        }
    }];
Owner

casademora commented Jan 15, 2013

If you're inside an NSOperation at this point, adding a new context here seems a bit more than necessary.
Also, find or create like this is going to be expensive all the time. Every NSFetchRequest goes down to the persistent store...what you want to do is:

  1. gather all your URL hashes in a set or array
  2. fetch all your CustomObject entities at once,
  3. look through that result list and check for existence there

it might be more code, but it's more performant.

On Jan 15, 2013, at 10:51 AM, Clarisse Laurier notifications@github.com wrote:

Sure, here

[MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
for (NSDictionary *dict in dictionary) {
if (_isCancelled == NO) { //This is part of an NSOperation
NSString *url = [dict valueForKey:@"Link"]; // This is an url
NSNumber *hash = [NSNumber numberWithInteger:url.hash]; //I Hash the url to get something unique

            // This is the method where I removed "inContext:localContext"
            CustomObject *mds = [CustomObject MR_findFirstByAttribute:@"objectHash" withValue:hash];
            if (mds) {
                //DDLogVerbose(@"Object exists already : %@", mds.title);
            }
            else {
                //DDLogVerbose(@"Creating Object ...");
                mds = [CustomObject MR_createInContext:localContext];
                mds.objectHash = hash;
                mds.title = [dict valueForKey:@"Title"];
                mds.catalog = [NSNumber numberWithInteger:self.catalog];
                mds.link = url;
            }
        }
    }
} completion:^(BOOL success, NSError *error) {
    if (success) {
          //Not relevant stuff
    }
    else {
        DDLogError(@"-- AN ERROR OCCURED WHILE SAVING :%@ --", error);
    }
}];


Reply to this email directly or view it on GitHub.

I'll try that thank you !
But I still don't understand why adding "inContext:localContext" makes it 10 times slower ! :p

EDIT : Also I'm having a hard time coming up with a predicate that would be able to fetch all the items (part 2) ) that would have the hash attribute corresponding from any object of the hash array (Don't know if I'm explaining myself well enough :p)

I'm still reading NSPredicate documentation or filtering array questions on StackOverflow but can't find what I want to do the filtering part :p

EDIT : Now I think I misunderstood what you mean, I think I don't need any predicate ..
EDIT 2 : Okay I finally managed to do it as you said, the benefit is small (less than a second on iphone 5 and a bit more on 3GS) but still is better thanks !

Owner

casademora commented Jan 16, 2013

you probably want something like:

[NSPredicate predicateWithFormat:@"urlHash in %@", collectionOfURLhashes];

and apply that to the entity with the property urlHash (otherwise it'll crash on not finding the key path)...

Saul Mora
@casademora
saul@casademora.com

On Wednesday, January 16, 2013 at 3:39 AM, Clarisse Laurier wrote:

I'm still reading NSPredicate documentation or filtering array questions on StackOverflow but can't find what I want to do the filtering part :p


Reply to this email directly or view it on GitHub (#386 (comment)).

Thank you very much !

Completely forgot to close, sorry about that !

ClarisseL closed this Feb 20, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment