Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

UICollectionView reloadData seems slow/laggy (not instantaneous)

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.

like image 974
rebello95 Avatar asked Sep 02 '14 00:09

rebello95


2 Answers

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)

like image 70
Thomas Müller Avatar answered Sep 20 '22 22:09

Thomas Müller


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.
like image 31
Aaron Brager Avatar answered Sep 17 '22 22:09

Aaron Brager