5

Hello: I'm using a collection view within my app, and I've noticed that it's taking longer than expected to refresh using reloadData. My collection view has 1 section, and I'm testing it with 5 cells (each of which has 2 buttons and a label). I put some logs into my code to show how long the system is actually taking to refresh. Interestingly enough, the logs indicate that it's refreshing faster than it is. On a device, for example, it will take up to ~0.2sec (noticeable), but here are the logs:

0.007s From the time reloadData is called to the time cellForItemAtIndexPath is called the first time

0.002s Per cell to load and be returned

0.041s From the time reloadData is called to the time where cell #5 is returned

There isn't anything particularly intensive in the cellForItemAtIndexPath function (basically just finds a dictionary with 3 values within an NSArray at the indexPath's row). Even when I removed this and just returned a cell with a blank button, I saw the same behavior, however.

Does anyone have any idea as to why this may be happening? It's only happening on a physical device (iPad Air), by the way. Thanks!

EDIT #1

Per @brian-nickel's comment, I used the Time Profiler instrument, and found that it does indeed spike each time reloadData is called. Here's a screenshot:

Time Profiler

@ArtSabintsev, here is the function surrounding the reloadData call, followed by the cellForItemAtIndexPath:

//Arrays were just reset, load new data into them
//Loop through each team
for (NSString *team in moveUnitsView.teamsDisplaying) { //CURRENT TEAM WILL COME FIRST

    //Create an array for this team
    NSMutableArray *teamArr = [NSMutableArray new];

    //Loop through all units
    for (int i = [Universal units]; i > 0; i--) {

        //Set the unit type to a string
        NSString *unitType = [Universal unitWithTag:i];

        //Get counts depending on the team
        if ([team isEqualToString:currentTeam.text]) {

            //Get the number of units of this type so that it supports units on transports. If the territory is a sea territory and the current unit is a ground unit, check the units in the transports instead of normal units
            int unitCount = (ter.isSeaTerritory && (i == 1 || i == 2 || i == 8)) ? [self sumOfUnitsInTransportsOfType:unitType onTerritory:ter onTeam:team] : [ter sumOfUnitsOfType:unitType onTeam:team];

            //Get the number of movable units on this territory
            int movableCount = 0;
            if (queue.selectedTerr != nil && queue.selectedTerr != ter) { //This is here to prevent the user from selecting units on another territory while moving units from one territory
                movableCount = 0;
            } else if (ter.isSeaTerritory && (i == 1 || i == 2 || i == 8)) { //Units on transports - can be an enemy territory
                movableCount = [self sumOfUnitsInTransportsOfType:unitType onTerritory:ter onTeam:team];
            } else if ([Universal allianceExistsBetweenTeam:team andTeam:ter.currentOwner] || i == 3 || i == 9) { //Other units - only planes can be on an enemy territory
                movableCount = [ter sumOfMovableUnitsOfType:unitType onTeam:team];
            }

            //See if there are units of this type on this territory on this team
            if (unitCount > 0) {

                //Add data to this team's dictionary
                NSMutableDictionary *unitInfo = [NSMutableDictionary new];
                [unitInfo setObject:@(i) forKey:@"UnitTag"];
                [unitInfo setObject:unitType forKey:@"UnitType"];
                [unitInfo setObject:@(unitCount) forKey:@"Count"];
                [unitInfo setObject:@(movableCount) forKey:@"MovableCount"];
                [unitInfo setObject:team forKey:@"Team"];

                //Add the dictionary
                [teamArr addObject:unitInfo];

                //Increment the counter
                if (unitsOnCT) { //Must check or it could cause a crash
                    *unitsOnCT += 1;
                }
            }
        }
    }

    //Add the team array
    [moveUnitsView.unitData addObject:teamArr];
}

//Reload the data in the collection view
[moveUnitsView.collectionV reloadData];

And my cellForItemAtIndexPath's relevant code:

    //Dequeue a cell
    UnitSelectionCell *cell = [collectionView dequeueReusableCellWithReuseIdentifier:@"UnitSelectionCell" forIndexPath:indexPath];

    //Get the team array (at the index of the section), then the unit's data (at the index of the row)
    NSMutableDictionary *unitData = (moveUnitsView.unitData[indexPath.section])[indexPath.row];

    //Get values
    int unitTag = [[unitData objectForKey:@"UnitTag"] intValue];
    int count = [[unitData objectForKey:@"Count"] intValue];
    int movableCount = [[unitData objectForKey:@"MovableCount"] intValue];
    NSString *unitType = [unitData objectForKey:@"UnitType"];

    //Set the cell's values
    [cell.upB addTarget:self action:@selector(upMoveUnits:) forControlEvents:UIControlEventTouchUpInside]; [cell.upB setTag:unitTag];
    [cell.iconB setBackgroundImage:[UIImage imageWithContentsOfFile:[[NSBundle mainBundle] pathForResource:[Universal imageNameForUnit:unitType team:[unitData objectForKey:@"Team"]] ofType:nil]] forState:UIControlStateNormal];
    [cell.iconB setTitle:[Universal strForExpDisplay:count] forState:UIControlStateNormal];
    [Universal adjustTitlePlacementOfB:cell.iconB autosize:FALSE]; //Don't autosize because this is a collection view
    cell.unitTypeL.text = unitType;
    cell.unitTypeL.adjustsFontSizeToFitWidth = cell.unitTypeL.adjustsLetterSpacingToFitWidth = TRUE;

    //Set fonts
    [Universal setFontForSubviewsOfView:cell];

    //Return the cell
    return cell;

When the collection view is initialized, cells are registered using:

[moveUnitsView.collectionV registerNib:[UINib nibWithNibName:@"UnitSelectionCell" bundle:nil] forCellWithReuseIdentifier:@"UnitSelectionCell"];

EDIT #2

@roycable and @aaron-brager pointed out that this could be caused by using imageWithContentsOfFile:. To test this out, I changed cellForItemAtIndexPath to this:

    //Dequeue a cell
    UnitSelectionCell *cell = [collectionView dequeueReusableCellWithReuseIdentifier:@"UnitSelectionCell" forIndexPath:indexPath];

    //Get the team array (at the index of the section), then the unit's data (at the index of the row)
    NSMutableDictionary *unitData = (moveUnitsView.unitData[indexPath.section])[indexPath.row];

    //Get values
    int unitTag = [[unitData objectForKey:@"UnitTag"] intValue];

    [cell setBackgroundColor:[UIColor redColor]];
    [cell.upB removeTarget:nil action:NULL forControlEvents:UIControlEventTouchUpInside];
    [cell.upB addTarget:self action:@selector(upMoveUnits:) forControlEvents:UIControlEventTouchUpInside]; [cell.upB setTag:unitTag];

    //Return the cell
    return cell;

Strangely, this doesn't seem to fix the issue. It's literally doing no intensive tasks in that function but it still seems to be lagging (and the Time Profiler seems to confirm that).

In response to the requests for code from Universal, instead of posting code I'll just summarize it:

+units just returns 17

+unitWithTag: uses a switch to return an NSString corresponding to a number between 1-17

+allianceExistsBetweenTeam: sees if an array contains one of the strings

+setFontForSubviewsOfView: is a recursive function that basically uses this code

Unfortunately, this doesn't seem very relevant since the issue is still occurring with the oversimplified cellForItemAtIndexPath function.

I also implemented @aaron-brager's new suggestions. I removed the target before adding a new one, and I made the changes to Time Profiler. I didn't see anything really pop out... Here's the screenshot. Everything related to UIImage is irrelevant to this question, as is NSKeyedArchiver, so the only other things that really make sense are strings, arrays, and dictionaries:

Time Profiler 2

Any and all help is greatly appreciated - I really need to get this fixed (hence the bounty). Thank you!

Edit #3 - Solution identified

So, it turns out that the issue wasn't in either of those functions. The issue was the function (let's call it Function A) that called the update function above (let's call it Function B). Right after Function A called Function B, it performed a CPU-intensive task. I wasn't aware of the fact that reloadData is at least partially asynchronous, so I'm assuming the CPU-intensive task and reloadData ended up racing for CPU time. I solved my problem by adding the following right before return cell;:

    if (indexPath.row == [self collectionView:collectionView numberOfItemsInSection:indexPath.section] - 1) {

        [self performSelector:@selector(performMyCPUIntensiveTask:) withObject:myObject afterDelay:0.1];
    }

I hope this helps someone else in the future. Thank you to everyone who helped, I sincerely appreciate it.

Community
  • 1
  • 1
rebello95
  • 8,486
  • 5
  • 44
  • 65
  • Have you used the [Time Profiler](https://developer.apple.com/Library/ios/documentation/AnalysisTools/Reference/Instruments_User_Reference/TimeProfilerInstrument/TimeProfilerInstrument.html)? If you're experiencing a 200ms delay you should see a nice big spike after reloading. – Brian Nickel Sep 04 '14 at 00:41
  • Would you mind providing us with all of your sample code? Also, are you using reusable cells? – ArtSabintsev Sep 04 '14 at 00:46
  • Thanks for your responses, guys. I've added info in response to both of your comments - please see my edit. – rebello95 Sep 04 '14 at 00:57
  • Check show Objective-C only, and invert call tree, in the time profiler. This will give you a better idea of which code is taking up what amount of time. – Aaron Brager Sep 04 '14 at 01:55
  • 1
    I agree with @ArtSabintsev. You should show the code for your `Universal` methods. – Aaron Brager Sep 04 '14 at 01:55
  • I've had performance issues in the past with `UICollectionViews` because I was loaded images from disk in every `cellForItemAtIndexPath`, which I see you're also doing (`imageWithContentsOfFile:`). If you can cache the images so that it doesn't load from disk every time, that may help the reload. If not, it should at least help the scrolling speed in general. – roycable Sep 04 '14 at 01:56
  • On average, how large is `[Universal units]`? Also, I'm inclined to agree with @roycable. It may be the fact that you're loading images from disk. See if you can comment out various portions of the `cellForItemAtIndexPath:` code, and see which which portions cause the spike to appear when you hit reload. – ArtSabintsev Sep 04 '14 at 02:33
  • 1
    Thanks guys. I've tried all the things that were just mentioned, and have updated my question. `[Universal units]` is a very simple function that contains one line: `return 17;`. Regarding image loading, please see my updated question. – rebello95 Sep 04 '14 at 02:40
  • So, this should be asked for the sake of being asked, but are you using Xcode 5 or Xcode 6 beta? – ArtSabintsev Sep 04 '14 at 02:43
  • I'm using Xcode 5, testing on an iPad Air running the latest version of iOS 7. There's no lag on the simulator, though. – rebello95 Sep 04 '14 at 02:44
  • Oh shoot... I think I figured it out. There's a CPU-intensive function that gets called right after the function that calls `reloadData` (the one shown above). When I comment out that function call the lag vanishes. Can someone explain why this would be the cause even though the intensive function is being called *after* reloading the data? – rebello95 Sep 04 '14 at 02:47
  • I solved the problem. If anyone in the future is looking for a solution, check out @aaron-brager's answer below and my third edit above. Thank you to everyone who helped! – rebello95 Sep 04 '14 at 03:39

2 Answers2

11

Make sure you're on the main thread when you call reloadData.

NSLog("on main thread: %@", [NSThread isMainThread] ? @"YES" : @"NO");

If you're not then use GCD to send the message on the main thread:

dispatch_async(dispatch_get_main_queue(), ^{
    [moveUnitsView.collectionV reloadData];
});

(Not 100% sure about syntax, I just typed this into the browser)

Thomas Müller
  • 15,565
  • 6
  • 41
  • 47
  • Just confirmed that this is indeed running *on* the main thread. Thanks though. – rebello95 Sep 04 '14 at 02:38
  • 1
    For me in Swift, it was taking several seconds from calling reloadData untill the collectionview would refresh. This solved it. – t0PPy Mar 22 '16 at 22:03
5

Some possibilities:

  • Your presumably recursive function to set the fonts is probably expensive.
  • A few of the other Universal functions look like they might be expensive.
  • It does not appear that you ever remove the button target and every time a cell is reused, you are adding additional targets to it.
  • imageWithContentsOfFile: skips the cache; use imageNamed: instead.
Aaron Brager
  • 65,323
  • 19
  • 161
  • 287
  • +1, accepted, and +bounty (after the time limit lets me) for all your help even though the source of the problem was my stupidity in another function. Thank you! – rebello95 Sep 04 '14 at 02:55