Optimizing iOS method in objective-c

Question

I have such method and have a hard time optimizing it to run better. It takes roughly 1 sec with 100 units to get between NSLog(@"Debug2") and NSLog(@"Debug3"). And 2 sec on iPhone.

Method idea:

Input:

• NSMutableArray times - containing times (2012-12-12, 2013-01-19) and etc
• NSMutableArray objectArray - contains LogUnit objects.

Output: Array of computed views.

Main idea: a view for each time with objects from objectArray whose time is the same.

I have made it faster by showing a small proportion of objects (see counted in code and _breakPlease), yet it's still not fast enough.

- (NSMutableArray*) sortViews: (NSMutableArray *)times and:(NSMutableArray *)objectArray
{
    NSLog(@"debug2");
    NSMutableArray *views = [[NSMutableArray alloc] initWithCapacity:times.count];

    NSString *number = [NSString stringWithFormat:@"SortLog%i ",[[NSUserDefaults standardUserDefaults] stringForKey:@"ObjectNumber"].intValue];

    NSString *comp = [[NSUserDefaults standardUserDefaults] stringForKey:number];

    LogUnit *unit;

    int counted = 0;
    for(int x = 0; x != times.count; x++)
    {
        @autoreleasepool {

        UIView *foo = [[UIView alloc] initWithFrame:CGRectMake(0, 0, 320, 400)];   
        foo.backgroundColor = [UIColor clearColor];

        int f = 0;
        int h = 0;

        NSString *attributeName = @"realTime";
        NSPredicate *predicate = [NSPredicate predicateWithFormat:@"%K IN %@", attributeName, [times objectAtIndex:x]];
        // Filter the objectArray by the time, so we don't have to run through all the objects in objectArray, and check if time is right.
        NSArray *filtered  = [objectArray filteredArrayUsingPredicate:predicate];

        for(int i = 0; i != filtered.count; i++)
        {
                unit = [filtered objectAtIndex:i];

                // Filter units by user choice (comp). Like comp = Warnings or comp = Errors and etc.
                if([[unit status] isEqualToString:comp] || [comp isEqualToString:NULL] || comp == NULL)
                {
                     // testing if the unit is correct to use
                    if([unit getEvent] != NULL)
                    {
                    // below is some computation for frames, to get them to proper position
                        unit.view.frame = CGRectMake(unit.view.frame.origin.x, f * unit.view.frame.size.height + 30, unit.view.frame.size.width, unit.view.frame.size.height);
                        [foo addSubview:unit.view];
                        counted++;
                        f++;
                        h = unit.view.frame.size.height * f;
                    }
                }
        }

        UILabel *myLabel = [[UILabel alloc] initWithFrame:CGRectMake(0, 0, 320, 30)];
        myLabel.backgroundColor = [UIColor clearColor];
        myLabel.font = [UIFont boldSystemFontOfSize:20];
        myLabel.textColor = [UIColor colorWithRed:88 green:154 blue:251 alpha:1];
        myLabel.textAlignment = UITextAlignmentCenter;
        myLabel.text = [times objectAtIndex:x];
        // Just adding a label with "2012-12-30" or etc on top of the view.    
        [foo addSubview:myLabel];

        foo.frame = CGRectMake(0,0, 320, h + 30 );
        h = 0;
        [views addObject:foo];

        // counting added views for performance, if more than 30, return, and show only small portion of whole units, user has the option to show all the units if he wants to, but that takes a time to load..
        if(counted > 30 && filtered.count != counted)
        {
            if(_breakPlease == false)
            {
                _broken = true;
                break;
            }
        }
       }
    }
    NSLog(@"debug3");
    return views;
}

Answer 1

times.count in for(int x = 0; x != times.count; x++) is a method call, move it to a variable. Also, you should probably use the < instead of != as it is more common.

However, as you don't really need the index of the element, you can replace both iterations

for(int i = 0; i != filtered.count; i++) {
    unit = [filtered objectAtIndex:i];

by native iteration

for (LogUnit* unit in filtered) {

Is @autoreleasepool necessary in every iteration? Having it outside for should help performance as well. In most application you are not required to use @autoreleasepool at all.

NSString *attributeName = @"realTime";
NSPredicate *predicate = [NSPredicate predicateWithFormat:@"%K IN %@", attributeName, [times objectAtIndex:x]];
// Filter the objectArray by the time, so we don't have to run through all the objects in objectArray, and check if time is right.
NSArray *filtered  = [objectArray filteredArrayUsingPredicate:predicate];

Your filtering doesn't do what your comment describes. You don't want to run through all the objects but that's exactly the thing filteredArrayUsingPredicate: does. Only using reflexion and moving the objects into a new array. By using the filtering code, you are losing performance.

There are various code quality issues not related to performance:

1. usage of variable names x, f, h, objectArray. Use more descriptive names or use i, j, k for iterators.

2. Move some code to separate methods, e.g. view generation.

3. Don't use NULL in Obj-C, use nil. NULL is to be used only when you interact with C pointers, not Obj-C objects.

4. [comp isEqualToString:NULL] will never evaluate to YES (true)

5. if(_breakPlease == false) Don't use false in Obj-C. Use YES or NO. Never ever compare a BOOL with a bool literal. Use logical operators instead, e.g. if (!_breakPlease).

Algorithm performance:

The main problem of your algorithm is its complexity. Basically you have two arrays and you compare objects from an array with all objects from the other array. That's O(n^2)

There are many ways how you could improve this, I will list some of them:

1. Don't use a NSArray to store the log units. Use a NSDictionary where time is the key and log unit, or possibly array of log units with the same time, is the value. Finding log units with the given time will be trivial then.

2. Sort times and objectArray by time. You can then iterate through both arrays at the same time and you get O(n + n * log n).

Answer 2

if you are looping over a collection, you should use enumeration. This could be fast enumeration (as shown by Sulthan for (LogUnit* unit in filtered)) or even better block based enumeration.

[filtered enumerateObjectsUsingBlock:^(LogUnit *unit, NSUInteger idx, BOOL *stop) {
    //…
}];

This has two one advantages over fast enumeration:

• it is even faster (on stack overflow some say ~15%)_{this was misinformation}
• You get also an index passed into the block, that you can use for calculating frames and similar

further benefits of enumeration in general

• The enumeration is more efficient than using NSEnumerator directly.
• The syntax is concise.
• The enumerator raises an exception if you modify the collection while enumerating.
• You can perform multiple enumerations concurrently.

apple's doc: Enumeration: Traversing a Collection’s Elements