stringByAppendingString causing system to run out of memory for applications

Question

Can someone point me in the direction why my system tells me I'm running out of memory for applications when I run the following code over a "large" (37000 rows) text file?

-(void) writeToFile: (NSString*)filePath withSeparator:(NSString*) fieldSep{
NSString* completeFile = [[[NSString alloc] initWithString:@""] autorelease];
for(int i=0;i<[self numberOfRows];i++){
    printf("im at line number... %i of %i\n",i,[self numberOfRows]);
    for(int j=0;j<[self numberOfColumns];j++){
        completeFile = [completeFile stringByAppendingString:[self objectInRow:i column:j]];

        if(j<[self numberOfColumns]-1){
            //separator for all columns except last one
            completeFile = [completeFile stringByAppendingString:fieldSep];
        }
    }
        completeFile = [completeFile stringByAppendingString:@"\n"];
}
NSError *error = nil;
[completeFile writeToFile:filePath atomically:NO
                 encoding:NSStringEncodingConversionAllowLossy error:&error];
if(error){
    NSLog(@"Error writing file at %@\n%@",
          filePath, [error localizedFailureReason]);
}

}

I've added the printf for debugging reasons, first 4000 rows seem to happen instantly, then it slowly slows down... my file contains over 37000 rows similar to these:

1893-11-6   136 194 165

Answer 1

When you allocate objects using factory methods, the objects are added to the autoreleasepool. The autoreleasepool is only drained when your event loop runs, after your IBAction has returned.

The trick here would be to put the contents of your loop in its own autorelasepool.

But let's address the biggest problem first. There's a NSMutableString class that you should use here, which would drastically reduce the number of objects you need to create.

We'll switch completeFile to a NSMutableString, constructed using a factory method, then append to it:

-(void) writeToFile: (NSString*)filePath withSeparator:(NSString*) fieldSep{
    NSMutableString* completeFile = [NSMutableString string];
    for(int i=0;i<[self numberOfRows];i++){
        printf("im at line number... %i of %i\n",i,[self numberOfRows]);
        for(int j=0;j<[self numberOfColumns];j++){
            [completeFile appendString:[self objectInRow:i column:j]];

            if(j<[self numberOfColumns]-1){
                //separator for all columns except last one
                completeFile appendString:fieldSep];
            }
        }
            [completeFile appendString:@"\n"];
    }
    NSError *error = nil;
    [completeFile writeToFile:filePath atomically:NO
                     encoding:NSStringEncodingConversionAllowLossy error:&error];
    if(error){
        NSLog(@"Error writing file at %@\n%@",
              filePath, [error localizedFailureReason]);
    }
}

This leaves another problem, though. See that [self objectInRow:i column:j]? It's still (presumably) an autoreleased object. That won't get cleaned up.

We may have made your code run without crashing, depending on the size of the data, but it's a question of when it crashes not if.

To fix this, we need to introduce autoreleasepools. Let's do one per row and per column. This might seem excessive (and, indeed, it is in this case since we've eliminated autoreleasepool use in the outer loop) but autoreleasepools are pretty cheap. If you're doing a loop over a large amount of data, it's just good practice.

You can replace each of your for blocks with @autorelease blocks, for instance:

for(int i=0;i<[self numberOfRows];i++){

With:

for(int i=0;i<[self numberOfRows];i++) @autoreleasepool {

That gives us this code:

-(void) writeToFile: (NSString*)filePath withSeparator:(NSString*) fieldSep{
    NSMutableString* completeFile = [NSMutableString string];
    for(int i=0;i<[self numberOfRows];i++) @autoreleasepool {
        printf("im at line number... %i of %i\n",i,[self numberOfRows]);
        for(int j=0;j<[self numberOfColumns];j++) @autoreleasepool {
            [completeFile appendString:[self objectInRow:i column:j]];

            if(j<[self numberOfColumns]-1){
                //separator for all columns except last one
                completeFile appendString:fieldSep];
            }
        }
            [completeFile appendString:@"\n"];
    }
    NSError *error = nil;
    [completeFile writeToFile:filePath atomically:NO
                     encoding:NSStringEncodingConversionAllowLossy error:&error];
    if(error){
        NSLog(@"Error writing file at %@\n%@",
              filePath, [error localizedFailureReason]);
    }
}

A final note, though. Your error check here is not safe. What happens to an error pointer passed in like this on success isn't defined.

    [completeFile writeToFile:filePath atomically:NO
                     encoding:NSStringEncodingConversionAllowLossy error:&error];
    if(error){
        NSLog(@"Error writing file at %@\n%@",
              filePath, [error localizedFailureReason]);
    }

Instead, you want this:

    BOOL ok = [completeFile writeToFile:filePath atomically:NO
                     encoding:NSStringEncodingConversionAllowLossy error:&error];
    if(!ok){
        NSLog(@"Error writing file at %@\n%@",
              filePath, [error localizedFailureReason]);
    }

This, then, should do what you want.

Answer 2

The problem is that each call of stringByAppendingString: creates a new autoreleased NSString objects. However, since the method continues in a loop, autorelease does not get a chance to free these objects.

You can solve it by adding an autorelease pool around the inner loop, like this:

for(int i=0;i<[self numberOfRows];i++){
    printf("im at line number... %i of %i\n",i,[self numberOfRows]);
    @autoreleasepool {
        for(int j=0;j<[self numberOfColumns];j++){
            completeFile = [completeFile stringByAppendingString:[self objectInRow:i column:j]];

            if(j<[self numberOfColumns]-1){
                //separator for all columns except last one
                completeFile = [completeFile stringByAppendingString:fieldSep];
            }
        }
    }
        completeFile = [completeFile stringByAppendingString:@"\n"];
}

Answer 3

You should use NSMutableString to append these strings.