Numerous duplicate URL tile requests

[dbainbridge]

trafficstars

It appears that if a tile is not already cached and must be requested it will often perform multiple URL requests for the same tile. I added an NSLog statement in the imageForTile:inCache: method in RMAbstractWebMapSource file:

NSHTTPURLResponse *response = nil;
NSLog(@"URL request: %@", [URLs objectAtIndex:0]);
NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:[URLs objectAtIndex:0]];
[request setTimeoutInterval:(self.requestTimeoutSeconds / (CGFloat)self.retryCount)];
image = [UIImage imageWithData:[NSURLConnection sendBrandedSynchronousRequest:request returningResponse:&response error:nil]];

I will get multiple statements (sometimes 10 for same tile) in the console such as the following:

URL request: http://a.tiles.mapbox.com/v3/examples.map-z2effxa8/5/5/5.png

I tested this using the MabBox example (on iPad 6.0 simulator).

I traced the issue to RMMapTileLayerView. In the method drawLayer:inContext:

@synchronized ([(RMAbstractWebMapSource *)_tileSource URLForTile:RMTileMake(x, y, zoom)])
{
    // this will return quicker if cached since above attempt, else block on fetch
    //
    if (_tileSource.isCacheable && [_tileSource imageForTile:RMTileMake(x, y, zoom) inCache:[_mapView tileCache]])
    {
        dispatch_async(dispatch_get_main_queue(), ^(void)
        {
            // do it all again for this tile, next time synchronously from cache
            //
            [self.layer setNeedsDisplayInRect:rect]; // <--- problem line
        });
    }
}

The setNeedsDisplayInRect call causes the drawLayer to be called numerous times for the same tile until it is actually downloaded and in the cache. Commenting out the setNeedsDisplayInRect fixes this problem and results in noticeable performance improvement (but may possibly result in other drawing issues in cases I am not aware of that this attempted to address to begin with).

What may be related: The URLForTile: method will return a new NSString object every time it is called even for the exact same tile so it is my understanding that the @synchronized call is more or less worthless in this instance.

[dbainbridge]

I see there was commit a week ago in the upstream project Route-Me to address the synchronized problem I mentioned above. This may also affect the numerous duplicate tile download problem. It would be better to understand though why it works correctly (at least on the simulator) by removing the setNeedsDisplayInRect alleviating the need for all that extra work done by the Route-Me commit mentioned above.

[brokedi]

This issue is still present. Are there any news about it?

The performance of the current original route-me Framework is better than in the MapBox Framework. The Tile Loading time is not fast.

[incanus]

We are currently doing some work that will possibly sidestep this entirely. Stay tuned.

[dbainbridge]

Can you give some indication on what kind of approach you are looking at?

[incanus]

We are looking into an OpenGL-based renderer that does the multithreaded tile fetching in a completely different way. So this goes deeper than just the network fetches.

[ghost]

I've seen a couple crashes in an app incorporating the MapBox iOS SDK (1.0.2 binary) which I think may be related to this issue. The problem only happens when I attempt to load a lot of uncached tiles on a retina iPad with a terrible wifi connection, but under those circumstances it seems fairly reproducible.

I think the crashes I saw could be related to this issue because the crash log had 60+ threads which seemed to be stuck inside of [NSURLConnection sendSynchronousRequest:returningResponse:error:] which conceivably could have resulted from the stuff including [NSURLConnection sendBrandedSynchronousRequest:request returningResponse:&response error:nil] which is mentioned in the first post of this issue.

Here is an excerpt of a the crash log I referred to. Threads 8-67 (not shown) appeared to be just the same as 7 and 68 (shown):

Thread 7 name:  Dispatch queue: com.apple.root.default-priority
Thread 7:
0   libsystem_kernel.dylib          0x3af61e30 mach_msg_trap + 20
1   libsystem_kernel.dylib          0x3af61fd0 mach_msg + 48
2   CoreFoundation                  0x32bc92b6 __CFRunLoopServiceMachPort + 126
3   CoreFoundation                  0x32bc802c __CFRunLoopRun + 900
4   CoreFoundation                  0x32b3b238 CFRunLoopRunSpecific + 352
5   CoreFoundation                  0x32b3b0c4 CFRunLoopRunInMode + 100
6   CFNetwork                       0x3289c612 CFURLConnectionSendSynchronousRequest + 330
7   Foundation                      0x335303da +[NSURLConnection sendSynchronousRequest:returningResponse:error:] + 242
8   mapmarkup2                      0x0009496a 0x6a000 + 174442
9   mapmarkup2                      0x000973c4 0x6a000 + 185284
10  mapmarkup2                      0x0009e492 0x6a000 + 214162
11  libdispatch.dylib               0x3ae98790 0x3ae97000 + 6032
12  libdispatch.dylib               0x3ae9c652 0x3ae97000 + 22098
13  libdispatch.dylib               0x3ae9c7d4 0x3ae97000 + 22484
14  libsystem_c.dylib               0x3aec07ee _pthread_wqthread + 358
15  libsystem_c.dylib               0x3aec0680 start_wqthread + 4

...

Thread 68 name:  Dispatch queue: com.apple.root.default-priority
Thread 68:
0   libsystem_kernel.dylib          0x3af61e30 mach_msg_trap + 20
1   libsystem_kernel.dylib          0x3af61fd0 mach_msg + 48
2   CoreFoundation                  0x32bc92b6 __CFRunLoopServiceMachPort + 126
3   CoreFoundation                  0x32bc802c __CFRunLoopRun + 900
4   CoreFoundation                  0x32b3b238 CFRunLoopRunSpecific + 352
5   CoreFoundation                  0x32b3b0c4 CFRunLoopRunInMode + 100
6   CFNetwork                       0x3289c612 CFURLConnectionSendSynchronousRequest + 330
7   Foundation                      0x335303da +[NSURLConnection sendSynchronousRequest:returningResponse:error:] + 242
8   mapmarkup2                      0x0009496a 0x6a000 + 174442
9   mapmarkup2                      0x000973c4 0x6a000 + 185284
10  mapmarkup2                      0x0009e492 0x6a000 + 214162
11  libdispatch.dylib               0x3ae98790 0x3ae97000 + 6032
12  libdispatch.dylib               0x3ae9c652 0x3ae97000 + 22098
13  libdispatch.dylib               0x3ae9c7d4 0x3ae97000 + 22484
14  libsystem_c.dylib               0x3aec07ee _pthread_wqthread + 358
15  libsystem_c.dylib               0x3aec0680 start_wqthread + 4

[dbainbridge]

Yes, this is what is happening. I addressed this issue in my own repo originally based on the MapBox framework by using a similar approach to the cache classes. I use a thread safe NSMutableSet to add the results of RMTileCacheHash(tile) to track which tiles have already been requested:

- (NSImage *)imageForTile:(RMTile)tile inCache:(RMTileCacheBase *)tileCache options:(RMImageForTileOptions)mask withBlock:(void (^)(NSImage *))imageBlock
{
    // verify we haven't already requested this tile
    NSNumber *tileCacheHash = RMTileCacheHash(tile);
    if ([self containsObject:tileCacheHash]) {
        return [self imageForMissingTile:tile fromCache:tileCache];
    }
    else {
        [self addObject:tileCacheHash];
    }
.
.

My codebase however is for OS X and not iOS with other substantial refactoring (see imageBlock parameter above). I have no duplicate tiles being downloaded now.

[springmeyer]

/cc @springmeyer

[ghost]

@incanus, @springmeyer, and @mousebird... I'm thinking pretty seriously about forking develop to see if I can sort out the crashing issue with tile-fetching in the next month. Before I do that, I wanted to check if there is already an imminent solution to this in the works. Do any of you have an opinion about the probability of a fix for this making it into develop or release by early August? My impression is that tile fetching is on the list to get fixed along with Maply and vector tiles, and that's probably not going to make it into release until a fair ways down the road.

Assuming this wouldn't be a major duplication of effort, my basic plan is approximately:

1. Instrument the existing tile fetching code to measure how many individual tile fetches are initiated, how many http/https connections are required to fetch one tile (assuming this could be less than 1 if keepalive is used or more than 1 if there are lots of timeouts), and the average/min/max time required for fetching one tile.

2. Implement an alternate tile fetching mechanism (disabled by default), which fetches tiles asynchronously by way of an NSOperationQueue. This will try to use keep-alive if possible, but not pipelining.

3. Send you a pull request if the metrics from item 1 show that item 2 is a significant improvement.

Also, this is pure speculation, but for what it's worth, I wouldn't be surprised if the tile fetching issue becomes much worse when loading tiles over HTTPS with the 1.0.3 SDK. The difference would be due to the additional connection setup time required to complete an SSL handshake.

[incanus]

Hey @wsnook, sorry, long weekend offline. The Maply stuff is separate from vector tiles (which I've been experimenting with mostly on iOS 7 thus far) but Maply is up in a fork that you can try out here: https://github.com/mousebird/mapbox-ios-sdk/tree/maply Installation on this will get better very soon.

I would love to see some work around the non-Maply/CATiledLayer version, though, as this would also benefit the upstream Alpstein project and just be good in general. I've not been working on the current implementation lately, just these alternate approaches.

[ghost]

@incanus, Okay thanks, that's good to know. After spending a while digging through iOS 7 SDK docs over the weekend, I'm starting to think that perhaps I need to wait to act on this and re-evaluate once the new MapKit api is finalized.

[incanus]

Diminishing returns, right? :-)

[hhartz]

I'm getting the same issue here, it seems to happen in RMDatabaseCache: 269, because max operation count has been exceeded for the write queue, and this causes the image to not be added to the cache