Search code examples
iosobjective-csqliteuitableviewnsoperationqueue

NSOperationQueue blocks UITableView


The following code is by another employee at my company, but i have to solve the performance-issue now...

The method deleteObjectAndAllDependencies deletes the project and the related data from our SQLite DB...

The tableView isn't usable still everything is performed. How can I speed up this process? Without using functionality.

- (void)tableView:(UITableView *)tableView commitEditingStyle:(UITableViewCellEditingStyle)editingStyle forRowAtIndexPath:(NSIndexPath *)indexPath {

    if (editingStyle == UITableViewCellEditingStyleDelete) {

        if ([[SARAM RAM] selectedProjektIndex] == indexPath.row) [[SARAM RAM] setSelectedProjektIndex:0];

        NSBlockOperation *start = [NSBlockOperation blockOperationWithBlock:^{

            [[SAAufmassProject popFromDatabaseForGUID:[SARAM projektPool][indexPath.row]] deleteObjectAndAllDependencies];

            [[self projectData] removeObjectAtIndex:indexPath.row];
        }];

        NSBlockOperation *refresh = [NSBlockOperation blockOperationWithBlock:^{

            [tableView beginUpdates];

            [tableView deleteRowsAtIndexPaths:@[indexPath] withRowAnimation:UITableViewRowAnimationFade];

            [tableView endUpdates];

            [self dataHasBeenDeleted];
        }];

        [refresh addDependency:start];

        [[NSOperationQueue currentQueue] addOperation:start];
        [[NSOperationQueue currentQueue] addOperation:refresh];
    }
}

Solution

  • This code is curious. It creates two synchronous operations, making one dependent upon the other. It then proceeds to add these operations to the current queue (the main queue!). This is a tad inefficient and fails to achieve any performance benefits. Worse, it looks like someone had some ambition to make this code more responsive by employing asynchronous patterns, but didn't quite achieve this goal. (That might suggest that there was some deeper architectural problem that made this difficult.)

    Setting that aside, what you probably want is something that triggers an asynchronous update of the database while updating the model object and the UI in the main thread:

    - (void)tableView:(UITableView *)tableView commitEditingStyle:(UITableViewCellEditingStyle)editingStyle forRowAtIndexPath:(NSIndexPath *)indexPath {
    
        if (editingStyle == UITableViewCellEditingStyleDelete) {
    
            if ([[SARAM RAM] selectedProjektIndex] == indexPath.row) [[SARAM RAM] setSelectedProjektIndex:0];
    
            // get reference to the guid
    
            NSString *guid = [SARAM projektPool][indexPath.row]];
    
            // perform the deletion in some background queue
    
            [self.databaseQueue addOperationWithBlock:^{
                [[SAAufmassProject popFromDatabaseForGUID:guid] deleteObjectAndAllDependencies];
            }];
    
            // meanwhile, back on the ranch, we'll update the table view
    
            [[self projectData] removeObjectAtIndex:indexPath.row];
            [tableView deleteRowsAtIndexPaths:@[indexPath] withRowAnimation:UITableViewRowAnimationFade];
            [self dataHasBeenDeleted];
        }
    }
    

    The thing is, this raises a few questions:

    • Does your SQLite implementation support updates from background thread? Many simple implementations will not handle this correctly.

    • It makes me nervous that projektPool is using the same index that projectData is. Is the deleteObjectAndAllDependencies updating the former, while the latter is updated in this method. I suspect that this is going to be a problem.

    • I wonder what deeper interdependencies exist between between popFromDatabaseForGUID, deleteObjectAndAllDependencies, projektPool and projectData. I also wonder if these are truly thread-safe (e.g. when cellForRowAtIndexPath is called while this stuff goes on in the background, is the model and database in a consistent state)?

    Bottom line, to my earlier point that the existing code suggests that there were earlier attempts at concurrent processing, I'd be willing to bet that there are some deeper architectural problems that thwarted this endeavor. The problem is that we do not have enough here to diagnose the deeper issues (and I suspect that the design is far too complicated to convey that effectively in a forum like this).

    Assuming for a second that (a) there are truly deeper architectural issues; and (b) that the refactoring of this code is more than your want to contemplate, you might want to review the code to see if there are any tactical opportunities for performance improvement. For example, if deleteObjectAndAllDependencies is performing a lot of separate SQL statements to update the database, are you using transactions to speed this up? With SQLite, there can be a huge performance impact there.

    Bottom line, I might run this code through the "Time Profiler" tool in "Instruments" (including the "Record Waiting Threads" option), and track down the source of the performance problem. See if there might be some SQL-related tactical opportunities (e.g., transactions, indexes, etc.).