This loop chokes on 50,000 lines on the iPhone... can I improve it so it doesn't?

Question

FILE *file = fopen([gpsFilePath UTF8String], "r");
char c[1024];

while(fgets(c, 1024, file)!=NULL)
{
    NSString *cString = [[NSString alloc] initWithCString:c
                                                 encoding:NSMacOSRomanStringEncoding];

    NSArray *split = [cString componentsSeparatedByString:@","];
    if ([split count] != 3)
    {
        continue; //this should only happen on the first line
    }

    gpx = [gpx stringByAppendingString:[NSString stringWithFormat:@"      <trkpt lat=\"%@\" lon=\"%@\"></trkpt>\n\n", [split objectAtIndex:0], [split objectAtIndex:1]]];
 }

Answer 1

Can you use chunks bigger than 1024?

Answer 2

You're allocating several objects for each line of the file, and they aren't getting released because they're getting added to an autorelease pool, and the autorelease pool isn't getting a chance to drain. Add an autorelease pool that drains every some number of iterations:

NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
FILE *file = fopen([gpsFilePath UTF8String], "r");
char c[1024];
int line = 1;
while(fgets(c, 1024, file)!=NULL) {
  NSString *cString = [[NSString alloc] initWithCString:c encoding:NSMacOSRomanStringEncoding];
  NSArray *split = [cString componentsSeparatedByString:@","];
  if ([split count] != 3) { continue; }  //this should only happen on the first line
  gpx =  [gpx stringByAppendingString:[NSString stringWithFormat:@"      <trkpt lat=\"%@\" lon=\"%@\"></trkpt>\n\n",
                                      [split objectAtIndex:0],[split objectAtIndex:1]]];
  if(line % 1000 == 0)  // drain the pool every 1000 iterations
  {
    [pool release];
    pool = [[NSAutoreleasePool alloc] init];
  }
  line++;
 }
[pool release];

Answer 3

As others have pointed out, you are creating a lot of temporary objects. An awful lot. On top of that, the size of the temporary objects, at least gpx ones, is increasing with each pass of the loop. You might want to try something like:

NSMutableString *gpx = [NSMutableString string];
NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
for(NSString *line in [[NSString stringWithContentsOfFile:gpsFilePath usedEncoding:NULL error:NULL] componentsSeparatedByString:@"\n"]) {
  NSArray *split = [line componentsSeparatedByString:@","];
  [gpx appendFormat:@"      <trkpt lat=\"%@\" lon=\"%@\"></trkpt>\n\n", [split objectAtIndex:0], [split objectAtIndex:1]];
}
[pool release];
pool = NULL;

This example loads the contents of what's at gpsFilePath and splits it by new lines. Then, for each line, it splits the line on commas, and appends the results to the mutable string gpx. It wraps the processing part that creates lots of temporary objects in an autorelease pool so they get discarded as soon as possible.

At the end, the variable gpx will contain the processed results.

Answer 4

You are allocating cString without releasing or autoreleasing it. You should do a [cString release] each time when you're done with it.

Also, like the others said, you should use your own autorelease pool, and append to the existing gpx instead of creating a new string each time.