mapsplugin / cordova-plugin-googlemaps

Google Maps plugin for Cordova
Apache License 2.0
1.66k stars 913 forks source link

Urgent: map stop working with many remote markers #337

Closed hirbod closed 9 years ago

hirbod commented 9 years ago

Hi Masashi,

if I try to load more then 100 remote marker images, the map just stops working. From 50-80 it is ok and still fast, then suddently, it stops doing anything. If I set 6000 markers without image, everything works.

Just tested on iOS, I don't know how it would be on android. Please, could you have a look? I know, this problem will for sure be solved with marker cluster, but actually, it is a serious bug. I'm not sure what happen with locale marker images, but at least remote will die.. :(

hirbod commented 9 years ago

I'm not sure, but maybe it should load images in a background mode or something?

bildschirmfoto 2014-12-19 um 04 27 29 bildschirmfoto 2014-12-19 um 04 27 46 bildschirmfoto 2014-12-19 um 04 27 42 bildschirmfoto 2014-12-19 um 04 27 33 bildschirmfoto 2014-12-19 um 04 27 38

wf9a5m75 commented 9 years ago

I think there is a dead link (404 image).

robinbonnes commented 9 years ago

You could maybe try this:

map.addMarker({ position: p, icon: 'data:image/png;base64,[image as base64]' }, function(marker) {});

TheMassassian commented 9 years ago

Hi there,

I have a app that load up to 900 Markers with different images and it works. Do you resize the marker images?

hirbod commented 9 years ago

Yes, I do resize them. Actually there could not be any 404. The images are there - all of them. But even when it should be a 404, the plugin should handle this correctly.

I really do not have any idea, how to solve this. I will provide you guys my "fake" JSON file later, maybe you could debug better with this.

TheMassassian commented 9 years ago

Try do set the markers without resizing. I believe that is the problem. Am 20.12.2014 00:36 schrieb "Hirbod" notifications@github.com:

Yes, I do resize them. Actually there could not be any 404. The images are there - all of them. But even when it should be a 404, the plugin should handle this correctly.

— Reply to this email directly or view it on GitHub https://github.com/wf9a5m75/phonegap-googlemaps-plugin/issues/337#issuecomment-67712880 .

wf9a5m75 commented 9 years ago

Resizing the icon makes a new image data internally that domains some memory.

For example, loading 90 icon images with resizing means your app makes another small 90 icon images. This may cause memory leak, and app is going to be slow.

So if you use bunch of different images for icon, that should be resized in the server side.

On Sat, Dec 20, 2014, 6:08 AM TheMassassian notifications@github.com wrote:

Try do set the markers without resizing. I believe that is the problem. Am 20.12.2014 00:36 schrieb "Hirbod" notifications@github.com:

Yes, I do resize them. Actually there could not be any 404. The images are there - all of them. But even when it should be a 404, the plugin should handle this correctly.

— Reply to this email directly or view it on GitHub < https://github.com/wf9a5m75/phonegap-googlemaps-plugin/issues/337#issuecomment-67712880>

.

— Reply to this email directly or view it on GitHub https://github.com/wf9a5m75/phonegap-googlemaps-plugin/issues/337#issuecomment-67736814 .

hirbod commented 9 years ago

I'm afraid to tell you, but even when I do not resize, it will crash.

I just simulated more markers on my server (currently there are just 6 markers, I do some tricks with php)

            $users = $sql->getArray($qry);

            $users = array_merge($users, $users);
            $users = array_merge($users, $users);

            $users = array_merge($users, $users);
            $users = array_merge($users, $users); <-- if I add this one, its over, it will crash

Do I have any glitches in my code? Something that causes a memory leak?


          // markers
          var markers = $scope.getMarkers();

           // define array
            var points = [];

            for(var i=0;i<markers.length;i++) 
            {
                var obj = markers[i];

                // grab the first marker for circle drawing
                if(i == 0) {

                    var pos = new plugin.google.maps.LatLng(obj["lat"], obj["lng"]);
                    $rootScope.map.addCircle({
                        'center': pos,
                        'radius': 2500,
                        'strokeColor' : '#FFFFFF',
                        'strokeWidth': 3,
                        'fillColor' : 'rgba(255,145,1,0.3)'
                    });
                }

                console.log(obj['uid']);
                //return false;

                    //var attrName = key;
                    //var attrValue = obj[key];

                    var latLng = new plugin.google.maps.LatLng(obj["lat"], obj["lng"]);
                    points.push(latLng);

                    $rootScope.map.addMarker({

                        'position': latLng,
                        'uid': obj["uid"],
                        'firstname': obj["firstname"],
                        'picture': obj["picture"],
                        'isFavorite': obj["isFavorite"],
                        'isMe': obj["isMe"],
                        'picture_profile': obj["picture_profile"],
                        'icon': {
                            'url': obj["icon"],
                            'size': {
                                'width': 40,
                                'height': 50
                            }
                        },
                        'markerClick': function(marker) {
                            $scope.showUserOverlay(marker);
                        }     

                    });

            }

            //console.log(points);

            $scope.latLngBounds = new plugin.google.maps.LatLngBounds(points);

            $rootScope.map.moveCamera({
              'target' : $scope.latLngBounds
            });
hirbod commented 9 years ago

Just made some debugs:

As soon as I remove the "icon" url -> it works perfectly. This has nothing to do with the size of the icon. There must be a problem accessing so many remote markers...

wf9a5m75 commented 9 years ago

I see. Could you tell me the all icon urls?

wf9a5m75 commented 9 years ago

If you don't want to share the URL publicly, send me an e-mail please.

hirbod commented 9 years ago

Just sent you an email. Thanks!

hirbod commented 9 years ago

Happy new year to all of you guys. Sorry when I jumped on this so quickly but any news on this bug?

wf9a5m75 commented 9 years ago

@Hirbod I tried your dummy data (data_big.json), but I didn't face your error on my iPhone 5s. However, I received some memory warnings. So I think you got memory error.

In your code, at least, you need to improve your code. Holding the icon data of markers domain large memory. In order to reduce the memory usage, holding just the marker id (maybe uid in your code), then get the detail data when user click the marker.

For example:


map.addMarker({
  "position": new plugin.google.maps.LatLng(...),
  "icon": ...,
  "uid": data.uid,
  "markerClick": function(marker) {
     var uid = marker.get("uid");
     // ie. jQuery
     $.getJSON("http://yourserver/getDetail/" + uid, function(response) {
       alert(response);
     });
  }

Further more, using one function for the same process is better memory performance in Javascript. So, better the below code than the bottom one.

Better code:

map.addMarker({
  ...
  "markerClick": onMarkerClick
});

function onMarkerClick(marker) {
    ....
}

Bad code:

map.addMarker({
  ...
  "markerClick": function() {....}
})
hirbod commented 9 years ago

Hi Masashi,

thanks for the tips. I know, there is some potential to optimize, and I will incorporate your suggestions, but still, it crashes only when I use the external images. If I leave them to the defaults, the map will load, no matter how many markers I've used...

hirbod commented 9 years ago

Got the fix for this problem. It's not caused trough this plugin directly. Seems like this is the problem: https://issues.apache.org/jira/browse/CB-8002

actually cordova has a bug which creates a new iframe every .exec() call is fired from bridge. As soon as 999+ iframes are reached, maps plugin crash. When I fire $('iframe').remove(), it works. I'm sure that this is my problem. This bug seems only to happen on 3.7.0+ and is fixed in 3.8.0.dev

For now (till 3.8.0) is out,

$('iframe').not(':last').remove();

fix most of my issues.

wf9a5m75 commented 9 years ago

Wow! I know Cordova creates iframe internally, but I didn't reach the idea of this. You are great, and congratulation!

wf9a5m75 commented 9 years ago

:100:

hirbod commented 9 years ago

I'm so sad.... but this fixes at least some other problems, but not this one. It still crash when I have to many remote markers... DAMN!

hirbod commented 9 years ago

SendDelegateMessage(NSInvocation ): delegate (webView:decidePolicyForNavigationAction:request:frame:decisionListener:) failed to return after waiting 10 seconds. main run loop mode: kCFRunLoopDefaultMode 2015-01-24 03:24:51.712 Nachbarschaft[1052:19871] void SendDelegateMessage(NSInvocation ): delegate (webView:decidePolicyForNavigationAction:request:frame:decisionListener:) failed to return after waiting 10 seconds. main run loop mode: kCFRunLoopDefaultMode

event tried to add the remove stuff inside the addMarkers function.. im going crazy...

hirbod commented 9 years ago

I got some new / better logs.

2015-01-24 03:27:45.907 Nachbarschaft[1098:21122] void SendDelegateMessage(NSInvocation *): delegate (webView:resource:didFinishLoadingFromDataSource:) failed to return after waiting 10 seconds. main run loop mode: kCFRunLoopDefaultMode
2015-01-24 03:27:46.653 Nachbarschaft[1098:21099] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '*** setObjectForKey: object cannot be nil (key: http://network.nachbarschaft.nightstomp.com/user/pictures/8/marker_pin.png?24012015)'
*** First throw call stack:
(0x185f1259c 0x1966680e4 0x185dfd1f8 0x100058538 0x100ab8df0 0x100ac28c8 0x100058368 0x100053ef8 0x186de5df0 0x185eca9ec 0x185ec9c90 0x185ec7d40 0x185df50a4 0x18ef975a4 0x18a72a3c0 0x100010604 0x196cd6a08)
libc++abi.dylib: terminating with uncaught exception of type NSException

This seems to be something interessting: 2015-01-24 03:27:46.653 Nachbarschaft[1098:21099] * Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '* setObjectForKey: object cannot be nil (key: http://network.nachbarschaft.nightstomp.com/user/pictures/8/marker_pin.png?24012015)'

Could this help you to improve your code? http://stackoverflow.com/questions/15280176/failed-to-return-after-waiting-10-seconds-although-getting-a-bool-back

hirbod commented 9 years ago

When I have more then 60 markers with remote image... MAPS CRASH. 30.01 we want to submit the app to the appstore... I'm working 4 months on this App now..

My contractee is going to kill me when this app dies with 60 markers.. and I'm getting really nervous right now.

hirbod commented 9 years ago

Marker.m file:

changed

        dispatch_queue_t gueue = dispatch_queue_create("GoogleMap_addMarker", NULL);
        dispatch_sync(gueue, ^{

to

        //dispatch_queue_t gueue = dispatch_queue_create("GoogleMap_addMarker", NULL);
        dispatch_async(dispatch_get_main_queue(), ^{

AND DAMN YEAH. IT WORKS! Just need to download async, not sync. As Google SDK requeires all calls in the main thread, I've just called dispatch_get_main_queue() instead of gueue

600 Remote markers work, then:

2015-01-25 01:35:58.414 Nachbarschaft[38767:1142089] ((null)) was false: Reached the max number of texture atlases, can not allocate more.
2015-01-25 01:35:58.414 Nachbarschaft[38767:1142089] ((null)) was false: Sprite can't update, icon or texture is null (icon=0x0)
(lldb) 

Any feedback on this, Masashi?

wf9a5m75 commented 9 years ago

I'm in outside right now. My car gets an engine trouble (over heat).

On Sat, Jan 24, 2015, 4:40 PM Hirbod notifications@github.com wrote:

Marker.m file:

changed

    dispatch_queue_t gueue = dispatch_queue_create("GoogleMap_addMarker", NULL);
    dispatch_sync(gueue, ^{

to

    //dispatch_queue_t gueue = dispatch_queue_create("GoogleMap_addMarker", NULL);
    dispatch_async(dispatch_get_main_queue(), ^{

AND DAMN YEAH. IT WORKS! Just need to download async, not sync. As Google SDK requeires all calls in the main thread, I've just called dispatch_get_main_queue() instead of gueue

2000 Remote markers work, then:

2015-01-25 01:35:58.414 Nachbarschaft38767:1142089 was false: Reached the max number of texture atlases, can not allocate more. 2015-01-25 01:35:58.414 Nachbarschaft38767:1142089 was false: Sprite can't update, icon or texture is null (icon=0x0) (lldb)

Any feedback on this, Masashi?

— Reply to this email directly or view it on GitHub https://github.com/wf9a5m75/phonegap-googlemaps-plugin/issues/337#issuecomment-71344882 .

hirbod commented 9 years ago

http://stackoverflow.com/questions/25555778/optimising-custom-marker-image-performance-with-google-maps-sdk-for-ios

https://code.google.com/p/gmaps-api-issues/issues/detail?id=5756

So after 4 months of work... this plugin (or Google Maps SDK) is useless for me without Cluster... cause it can't display more then 500-600 (after my fix) markers.. (in a small radius of 2,5 miles)

hirbod commented 9 years ago

I still see some other bugs in Markers. Breakpoint exception.

          [self.iconCache setObject:data forKey:iconPath];

throwing: 'NSInvalidArgumentException', reason: '*\ setObjectForKey: object cannot be nil

hirbod commented 9 years ago

FINALLY I GOT IT.

Ok... there were a lot of logical bugs and many cases were data could not be set. I've investigated a lot of time and changed the code. I can load now up tp 1000-1500 markers till gMaps crashes because of

2015-01-25 01:35:58.414 Nachbarschaft[38767:1142089] ((null)) was false: Reached the max number of texture atlases, can not allocate more.

Things I did:

  1. Instead of dispatch_sync, rely on dispatch_async and attach to dispatch_get_main_queue()
  2. Never use NSData *data = [NSData dataWithContentsOfURL:url options:NSDataReadingMapped error:nil];as it is terrible slow and ends to memory leaks / result in broken downloads/data which will cause the NSInvalidArgumentException

-dataWithContentsOfURL:doesn’t return until the URL connection is over, which can take up to 2 minutes in the worst case (when the host isn’t available and the connection times out). Until that method returns, the UI is completely blocked. The user can’t do anything, the app won’t respond to any touches, including any for scrolling your tableview. Unacceptable.

  1. I've created a generic function inside of Markers.m
- (void)downloadImageWithURL:(NSURL *)url completionBlock:(void (^)(BOOL succeeded, UIImage *image))completionBlock
{
    NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:url];
    [NSURLConnection sendAsynchronousRequest:request
                                       queue:[NSOperationQueue mainQueue]
                           completionHandler:^(NSURLResponse *response, NSData *data, NSError *error) {
                               if ( !error )
                               {
                                   UIImage *image = [[UIImage alloc] initWithData:data];
                                   completionBlock(YES,image);
                               } else{
                                   completionBlock(NO,nil);
                               }
                           }];
}

which will do a Async request to download the markers. Last but not least:

      /***
       * Load the icon from over the internet
       */
      NSData *imgData = [self.iconCache objectForKey:iconPath];
      if (imgData != nil) {
        UIImage* image = [UIImage imageWithData:imgData];
        if (width && height) {
          image = [image resize:width height:height];
        }
        marker.icon = image;
        [self.commandDelegate sendPluginResult:pluginResult callbackId:callbackId];
      } else {
        //dispatch_queue_t gueue = dispatch_queue_create("GoogleMap_addMarker", NULL);
        dispatch_async(dispatch_get_main_queue(), ^{

          NSURL *url = [NSURL URLWithString:iconPath];
          //NSData *data = [NSData dataWithContentsOfURL:url options:NSDataReadingMapped error:nil];

            // download the image asynchronously
            [self downloadImageWithURL:url completionBlock:^(BOOL succeeded, UIImage *image) {
                if (succeeded) {

                    [self.iconCache setObject:image forKey:iconPath];

                    if (width && height) {
                        image = [image resize:width height:height];
                    }
                    marker.icon = image;

                    // The `anchor` property for the icon
                    if ([iconProperty valueForKey:@"anchor"]) {
                        NSArray *points = [iconProperty valueForKey:@"anchor"];
                        CGFloat anchorX = [[points objectAtIndex:0] floatValue] / image.size.width;
                        CGFloat anchorY = [[points objectAtIndex:1] floatValue] / image.size.height;
                        marker.groundAnchor = CGPointMake(anchorX, anchorY);
                    }

                    // The `infoWindowAnchor` property
                    if ([iconProperty valueForKey:@"infoWindowAnchor"]) {
                        NSArray *points = [iconProperty valueForKey:@"infoWindowAnchor"];
                        CGFloat anchorX = [[points objectAtIndex:0] floatValue] / image.size.width;
                        CGFloat anchorY = [[points objectAtIndex:1] floatValue] / image.size.height;
                        marker.infoWindowAnchor = CGPointMake(anchorX, anchorY);
                    }

                }
            }];

          [self.commandDelegate sendPluginResult:pluginResult callbackId:callbackId];
        });

I wrap inside a async dispatch and call the download sync. This let the map load much faster and download much faster then ever. Of course, if you have too many markers, you will first see the default marker icon which gets replaced trough the downloaded marker - but this behaviour is better I guess. Maybe you now (I'm really not an experienced Obj-C developer) how to improve this.

Next thing is to support markers from file:/// path, then we could download markers outside of the Maps Plugin and pass the internal file:/// documents AND tmp path. I know this should be possible to implement. Hope you can help after your car works fine again.

But the most important thing is the long awaited clusterer.

367

hirbod commented 9 years ago

And I think there is still more potential optimizing this: http://khanlou.com/2012/08/asynchronous-downloaded-images-with-caching/

But for now, my patch will fix the crashed caused trough your plugin, but will not fix the crashes caused trough Google Maps SDK.

But the caching method should be improved also.

Use https://github.com/FTW/FTWCache It stores Cache up to 7 days, this could be a problem, but I guess there a ways with md5 to prevent.

This article explains good.. http://khanlou.com/2012/08/asynchronous-downloaded-images-with-caching/

hirbod commented 9 years ago

Changed the code a little bit:

} else {
      /***
       * Load the icon from over the internet
       */
      NSData *imgData = [self.iconCache objectForKey:iconPath];
      if (imgData != nil) {
        UIImage* image = [UIImage imageWithData:imgData];
        if (width && height) {
          image = [image resize:width height:height];
        }
        marker.icon = image;
        [self.commandDelegate sendPluginResult:pluginResult callbackId:callbackId];
      } else {
        //dispatch_queue_t gueue = dispatch_queue_create("GoogleMap_addMarker", NULL);
        dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0ul);
        dispatch_async(queue, ^{

          NSURL *url = [NSURL URLWithString:iconPath];
          //NSData *data = [NSData dataWithContentsOfURL:url options:NSDataReadingMapped error:nil];

            // download the image asynchronously
            [self downloadImageWithURL:url completionBlock:^(BOOL succeeded, UIImage *image) {
                if (succeeded) {

                    if (width && height) {
                        image = [image resize:width height:height];
                    }

                    dispatch_async(dispatch_get_main_queue(), ^{
                        marker.icon = image;

                        // The `anchor` property for the icon
                        if ([iconProperty valueForKey:@"anchor"]) {
                            NSArray *points = [iconProperty valueForKey:@"anchor"];
                            CGFloat anchorX = [[points objectAtIndex:0] floatValue] / image.size.width;
                            CGFloat anchorY = [[points objectAtIndex:1] floatValue] / image.size.height;
                            marker.groundAnchor = CGPointMake(anchorX, anchorY);
                        }

                        // The `infoWindowAnchor` property
                        if ([iconProperty valueForKey:@"infoWindowAnchor"]) {
                            NSArray *points = [iconProperty valueForKey:@"infoWindowAnchor"];
                            CGFloat anchorX = [[points objectAtIndex:0] floatValue] / image.size.width;
                            CGFloat anchorY = [[points objectAtIndex:1] floatValue] / image.size.height;
                            marker.infoWindowAnchor = CGPointMake(anchorX, anchorY);
                        }

                        NSData *imgData = UIImagePNGRepresentation(image);
                        [self.iconCache setObject:imgData forKey:iconPath];

                        [self.commandDelegate sendPluginResult:pluginResult callbackId:callbackId];

                    });

                }
            }];

        });

      }
    }

I think it is a bit more performant now (High priority) but the iconCache seems not to work...

wf9a5m75 commented 9 years ago

It's midnight, and I finally got my private time. I'm reading your posts now.

wf9a5m75 commented 9 years ago

@Hirbod Thank you for great work, and your code is really good.

One point, the reason of dispatch_sync is that some one asked to me before for that. He wanted to hide markers until the icon images are loaded. And I agree with this idea.

For the reason, I will modify a little your code. Just a moment.

wf9a5m75 commented 9 years ago

modified

wf9a5m75 commented 9 years ago

@Hirbod As I mentioned, I added some code. But basically it's your great code. Thank you so much :100:

hirbod commented 9 years ago

Hi Masashi, wow, thanks for your reply and code modificaiton. Indeed, this is handled a bit better know. Still I don't understand isMapped (what is this for?) but the rest looks good to me.

Now just incoporate the watchdog and then please, release 1.2.5. I guess this optimization was something really important cause now the map will not block the UI anymore when loading images from remote.

Next step: better memory handling and optimization for image resizing. There are still better ways of doing (also keep aspect ratio, set better interpolarity Quality etc.)

I will do this next week for you and send you a PR, but this was - for now, the most important bug that has got fixed