Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance problems when adding many markers. #835

Closed
atannus opened this issue Feb 1, 2016 · 165 comments
Closed

Performance problems when adding many markers. #835

atannus opened this issue Feb 1, 2016 · 165 comments
Milestone

Comments

@atannus
Copy link

atannus commented Feb 1, 2016

I'm proposing a patch to the PluginMarker.java file that will greatly improve the performance when adding hundreds of markers to the map.

I propose the creation of a createMarkers method (note the plural), which will take the same marker definition as does the currently available createMarker, except for the fact they'll be in an array, therefore allowing the creation of multiple markers with a single over-the-bridge request. Naturally, the callback has to return an array of markers, instead of a single marker.

A private createMarker_ method is to be called by createMarkers. This does not break the current API, and the current createMarker method can be refactored to simply call the private version with its argument in an array.

I have already implemented marker caching and marker pre-loading which on top of that will drastically reduce memory consumption and prevent leaks.

This is where my understanding of this project ends. I don't know what my constraints as a developer are:

  • Are APIs frozen?
  • Are they meant to mimic their respective native environments?
  • Or abstract them away? (This preferred, imho.)
  • Do iOS and Android APIs diverge today?

I'm seeing fantastic performance with these improvements, I'd really like to contribute them.

Thanks a lot for the help.

@hirbod
Copy link
Contributor

hirbod commented Feb 1, 2016

The APIs aren't frozen, but I would like to have a heterogeneous environment. iOS and android should be the same level if possible. While the clustering branch resolves most of the memory problems, I would like to incorporate your suggestions, cause not everybody need clustering and the cluster branch has a lot less functionality currently. Once the funding is done, it will be released as own setup, but I would incorporate your edits there, too (ass the clustering can be deactivated by config).

So I'm open for everything. I'm not the lead developer of this plugin. Masashi was, but he dropped it and I took over maintenance and fixed bugs and merged good PRs and incorporated some fixes and functions by myself. So I'm very thankful if someone would help me here.

@atannus
Copy link
Author

atannus commented Feb 2, 2016

Can I see the clustering branch? Perhaps I should focus on these changes on that.

@hirbod
Copy link
Contributor

hirbod commented Feb 2, 2016

Sure. Are you on Skype? We should chat

@atannus
Copy link
Author

atannus commented Feb 3, 2016

I've though about this.

A cluster is just one marker where many would be, and it should be the end-user's job to cluster, not the plugin's. The user has the added benefit of being able to pick the clustering method which by the ways begs the question: which are yo using? Are you going to provide multiple? Or perhaps the injection of a clustering interface?

The questions on their own indicate that clustering should be handled outside the plugin. However you answer them, notice the plugin's clustering code will have to know about all the markers the user is creating, plus the clustering method the user wants to use and if the user wants a different clustering method they'll have to bake it in.

Clustering is a black box that takes a bunch of markers and returns one single marker. I would provide clustering as a factory method, to be used as:

var positions = [p1, p2, p3];
var clusterer = new plugin.google.maps.MarkerClusterer(opt_methodName);
var clusterPosition = clusterer.cluster(positions);

Where positions is an array of objects that expose lats and longs via properties or methods (i.e. objects that implement a certain Interface). Remember they're not likely to be the formal Marker objects, since we have not created them yet.

The clusterer will return one single such object exposing a lat-lng pair, which will be used to create the "real" Marker on the map.

This can take advantage of the changes I've made since the user can use the clusterer object to cluster multiple sets of positions outside the plugin then create all Markers (one for each cluster) at once.

We can also have it so that the user injects the clusterer as a function that takes markers and returns marker, both as described above. The user then calls some function with the data that to be clustered, and the function uses the injected clusterer. Once again, this puts into the plugin data that does not belong there.

These are just quick thoughts. Perhaps I miss the point entirely. Which way are you going?

@hirbod
Copy link
Contributor

hirbod commented Feb 3, 2016

Test it:

cordova plugin add https://github.com/mapsplugin/cordova-plugin-googlemaps#clusterer --variable API_KEY_FOR_ANDROID="YOURKEY" --variable API_KEY_FOR_IOS="YOURKEY"

Add following config object to .getMap()

                'controller': {
                    'clustering': true,
                    'rendering': 'animated',
                    'algorithm': 'nonHierarchicalDistanceBasedAlgorithm'
                }

eg

            $rootScope.map = plugin.google.maps.Map.getMap(mapDiv, {
                'controls': {
                    'compass': false,
                    'myLocationButton': false,
                    'indoorPicker': false,
                    'toolbar': false,
                    'zoom': false
                },
                'gestures': {
                    'scroll': true,
                    'tilt': true,
                    'rotate': true
                },
                'controller': {
                    'clustering': true,
                    'rendering': 'animated',
                    'algorithm': 'nonHierarchicalDistanceBasedAlgorithm'
                }
            });

Edit:
Btw, currently only nonHierarchicalDistanceBasedAlgorithm supported.

@hirbod
Copy link
Contributor

hirbod commented Feb 3, 2016

As soon as the user has zoomed enough, the real markers will be created.

bildschirmfoto 2016-02-03 um 16 35 02

Same for iOS
bildschirmfoto 2016-02-03 um 16 36 45

A real app to see it in action
https://itunes.apple.com/us/app/a-z-erdgastankstellen/id441500868?mt=8&ls=1
https://play.google.com/store/apps/details?id=de.lf.erdgas&hl=de

Not my app, but its the same codebase which is incorporated.

The Cluster branch works, but it is a bit hacky - and generally this plugin needs a lot refactoring (but I don't have time for that). If you have time and the mood to provide a better clustering solution, I'm open minded to follow your path and spending all donations to you, if you provide a better way for both systems.

@RegexLLC
Copy link

RegexLLC commented Feb 5, 2016

Without clustering, I'm using a mix of KML overlay for the bulk of the 'noise' and plain css markers placed over top of the mapdiv together.

@dbesiryan
Copy link

the clustering layers (orange markers) are showing wrong numbers. Always double of the real marker-value is shown. Any ideas?

@hirbod
Copy link
Contributor

hirbod commented Feb 7, 2016

dbesiryan - I'm not supporting something that isn't officially released. This branch is not finished

@Slavrix
Copy link

Slavrix commented Feb 9, 2016

I'd love to see this feature added. In an app I'm current working on, we have thousands of markers and the client doesn't want clustering.
On ios so far it has been working ok, but on Android it lags out the app as it draws them all (calling add marker for each one inside a foreach)
Deming the shore array to the plugin would be much better

@RegexLLC
Copy link

RegexLLC commented Feb 9, 2016

Could I maybe see your app that loads thousands of markers?

@Slavrix
Copy link

Slavrix commented Feb 18, 2016

I'll set-up a test app with how I'm doing it over the weekend. Sorry it took a while to respond.

@chrigi
Copy link
Contributor

chrigi commented Mar 3, 2016

@atannus Do you have a fork containing your changes? Have you implemented the bulk marker adding, caching and pre-loading for Android & iOS or only für Android?

@atannus
Copy link
Author

atannus commented Mar 3, 2016 via email

@chrigi
Copy link
Contributor

chrigi commented Mar 3, 2016

It appears so. There is a clusterer branch but I'm unsure about how complete that is. It would be great if you could commit your changes to your fork (and maybe even open a pull request) if you want to share your work.

@atannus
Copy link
Author

atannus commented Mar 3, 2016

I'll see what I can do.

On Thu, Mar 3, 2016 at 10:41 AM, Christian [email protected] wrote:

It appears so. There is a clusterer branch but I'm unsure about how
complete that is. It would be great if you could commit your changes to
your fork (and maybe even open a pull request) if you want to share your
work.


Reply to this email directly or view it on GitHub
#835 (comment)
.

André Tannús | Epungo | +55 11 2389-4360
We are a layer

@atannus
Copy link
Author

atannus commented Mar 3, 2016

I'm on this now.
Is there a suggested development flow?
IDE?
Should I edit the plugin while installed within a project or
change->(re)install->test?
Thanks.

On Thu, Mar 3, 2016 at 10:44 AM, Andre Tannus [email protected]
wrote:

I'll see what I can do.

On Thu, Mar 3, 2016 at 10:41 AM, Christian [email protected]
wrote:

It appears so. There is a clusterer branch but I'm unsure about how
complete that is. It would be great if you could commit your changes to
your fork (and maybe even open a pull request) if you want to share your
work.


Reply to this email directly or view it on GitHub
#835 (comment)
.

André Tannús | Epungo | +55 11 2389-4360
We are a layer

André Tannús | Epungo | +55 11 2389-4360
We are a layer

@hirbod
Copy link
Contributor

hirbod commented Mar 9, 2016

Well calling this a hostage is not very fine. And it's just for the cluster-branch. The cluster branch which is avaiable here "inoffically" was a paid job. It was just improved and cleaned by me. I just was about to give some money back to the code donator. If you guys help to finish this cluster branch, I'm open for it. The branch is not hidden, try it, fix it, improve it.

@atannus
Copy link
Author

atannus commented Mar 9, 2016

Is there a suggested development flow?

I have the plugin project checked out, so I have to make changes to that, uninstall/reinstall (so that the Cordova project picks up the changes), test, repeat. Also, (re)installing is a pain because the plugin name doesn't match the manifest name (or I'm doing something wrong).

Is there a better workflow than this? It's a pain...

Thanks.
AT

@cvaliere
Copy link

Hi @atannus

The development flow I would suggest is to edit the plugin while installed: use a very simple Ionic app that uses this map, run "ionic run android", then open the Android project into the IDE you like (eg. Android Studio) and edit directly in here with live reload allowed by Android Studio.

For clusters, I think you missed the point: the goal is to let the map decide, depending on the zoom level, to show individual markers or to cluster them. If you "create" the clusters outside the plugin, how is it supposed to work? Would you, for every zoom event, recalculate the clusters yourself?

Please tell me if I can help you

@atannus
Copy link
Author

atannus commented Mar 15, 2016

Thanks @cvaliere.

On Tue, Mar 15, 2016 at 11:19 AM, cvaliere [email protected] wrote:

Hi @atannus https://github.com/atannus

The development flow I would suggest is to edit the plugin while
installed: use a very simple Ionic app that uses this map, run "ionic run
android", then open the Android project into the SDK you like (eg. Android
Studio) and edit directly in here with live reload allowed by Android
Studio.

For clusters, I think you missed the point: the goal is to let the map
decide, depending on the zoom level, to show individual markers or to
cluster them. If you "create" the clusters outside the plugin, how is it
supposed to work? Would you, for every zoom event, recalculate the clusters
yourself?

Please tell me if I can help you


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:

#835 (comment)

André Tannús | Epungo | +55 11 2389-4360
We are a layer

@PapyElGringo
Copy link

Can you share the createMarkers method with us? I think it's would be a great evolution.

@cvaliere
Copy link

Hi @atannus

+1 for sharing createMarkers, I would definitely use it

thanks !

@atannus
Copy link
Author

atannus commented Mar 18, 2016

Hi guys.

Thanks for the support.

Aside from createMarkers, changes to the bridge call are also required. In
fact, I created a whole new bridge call for this performance issue. Also,
there's the need to keep track of the created markers, which I've done in
an experimental manner. I consider the code I wrote a hack up, and I'm
not comfortable sharing it just yet, especially since I did not bake in
support for the various different ways to provide de marker image.

Last week I put some time into making these changes decent enough to
contribute, but I could not finish the work.

I'll post something soon, please be patient.

On Fri, Mar 18, 2016 at 1:27 PM, cvaliere [email protected] wrote:

Hi @attanus

+1 for sharing createMarkers, I would definitely use it

thanks !


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#835 (comment)

André Tannús | Epungo | +55 11 2389-4360
We are a layer

@atannus
Copy link
Author

atannus commented Mar 21, 2016

Hi guys.

I've pushed the proposed changes to my fork, under branch develop/multiple-markers.
https://github.com/atannus/cordova-plugin-googlemaps/tree/develop/multiple-markers

Support is still very limited, and only tested with URL icons. You must preload icons in order to achieve the performance enhancement. If you don't, icons won't be cached and will be loaded individually, just as if you were adding markers one by one.

Here is some code that demonstrate the plugin in action. Remeber to change the icon paths to something real.

var map;
var icons = [
    "http://some.thing/path/to/image1.png"
];

document.addEventListener("deviceready", function () {

    // Initialize the map.
    var div = document.getElementById("map_canvas");
    map = plugin.google.maps.Map.getMap(div);
    var initPosition = new plugin.google.maps.LatLng(-23.548, -46.5745);
    map.addEventListener(plugin.google.maps.event.MAP_READY, function (map) {

        // Post-preload callback.
        var donePreloading = function () {
            // Animate camera.
            map.animateCamera({
                'target': initPosition,
                'zoom': 16,
                'duration': 1000
            },
                    function () {
                        placeMarkers();
                    })
        };

        // Preload images.
        map.preloadImages(icons, function (results) {
            donePreloading();
        });
    });
}, false);

placeMarkers = function () {
    var markerDefinitions = createMarkers();
    map.addMarkers(markerDefinitions, function (createdMarkers) {
        console.log(createdMarkers);
    });
}

createMarkers = function () {
    var markers = [];
    for (var i = 1; i < mockData.length; i++) {
        var mock = mockData[i];
        var position = new plugin.google.maps.LatLng(mock.lat, mock.lng);
        var markerDefinition = {
            'position': position
            , 'icon': {
                'url': icons[0],
                'size': {
                    'width': 32,
                    'height': 32
                }
            }
        };
        markers.push(markerDefinition);
    }
    return markers;
}

var mockData = [
    {
        lat: "-23.548922600000000",
        lng: "-46.574610800000000",
    },
    {
        lat: "-23.549337900000000",
        lng: "-46.574795600000000",
    },
    {
        lat: "-23.550828933700000",
        lng: "-46.576023101800000",
    },
    {
        lat: "-23.550889500000000",
        lng: "-46.576046300000000",
    },
    {
        lat: "-23.551199100000000",
        lng: "-46.575804400000000",
    },
    {
        lat: "-23.550761500000000",
        lng: "-46.575932700000000",
    },
    {
        lat: "-23.550820100000000",
        lng: "-46.575986100000000",
    },
    {
        lat: "-23.550958200000000",
        lng: "-46.576498800000000",
    },
    {
        lat: "-23.550552368200000",
        lng: "-46.574832916300000",
    },
    {
        lat: "-23.550529480000000",
        lng: "-46.575012207000000",
    },
    {
        lat: "-23.549200058000000",
        lng: "-46.573200225800000",
    },
    {
        lat: "-23.549949646000000",
        lng: "-46.572673797600000",
    },
    {
        lat: "-23.550093900000000",
        lng: "-46.572460600000000",
    },
    {
        lat: "-23.550067000000000",
        lng: "-46.572427400000000",
    },
    {
        lat: "-23.549314300000000",
        lng: "-46.571722400000000",
    },
    {
        lat: "-23.548916800000000",
        lng: "-46.572370800000000",
    },
    {
        lat: "-23.550040200000000",
        lng: "-46.571646100000000",
    },
    {
        lat: "-23.551219500000000",
        lng: "-46.571387300000000",
    },
    {
        lat: "-23.550374984700000",
        lng: "-46.571079254200000",
    },
    {
        lat: "-23.551379900000000",
        lng: "-46.572379600000000",
    },
    {
        lat: "-23.551279068000000",
        lng: "-46.571372985800000",
    },
    {
        lat: "-23.550819700000000",
        lng: "-46.571475300000000",
    },
    {
        lat: "-23.550776800000000",
        lng: "-46.571910200000000",
    },
    {
        lat: "-23.551599502600000",
        lng: "-46.575599670400000",
    },
    {
        lat: "-23.551599502600000",
        lng: "-46.575599670400000",
    },
    {
        lat: "-23.552324295000000",
        lng: "-46.575397491500000",
    },
    {
        lat: "-23.551766600000000",
        lng: "-46.575586100000000",
    },
    {
        lat: "-23.551599502600000",
        lng: "-46.575599670400000",
    },
    {
        lat: "-23.552499771100000",
        lng: "-46.574798584000000",
    },
    {
        lat: "-23.552565800000000",
        lng: "-46.574789700000000",
    },
    {
        lat: "-23.552565800000000",
        lng: "-46.574789700000000",
    },
    {
        lat: "-23.551799774200000",
        lng: "-46.573898315400000",
    },
    {
        lat: "-23.551788330100000",
        lng: "-46.574462890600000",
    },
    {
        lat: "-23.551799774200000",
        lng: "-46.573898315400000",
    },
    {
        lat: "-23.553100585900000",
        lng: "-46.575500488300000",
    },
    {
        lat: "-23.553400039700000",
        lng: "-46.575199127200000",
    },
    {
        lat: "-23.553100500000000",
        lng: "-46.574628900000000",
    },
    {
        lat: "-23.552700042700000",
        lng: "-46.573799133300000",
    },
    {
        lat: "-23.553236007700000",
        lng: "-46.574249267600000",
    },
    {
        lat: "-23.553236007700000",
        lng: "-46.574249267600000",
    },
    {
        lat: "-23.552700042700000",
        lng: "-46.573799133300000",
    },
    {
        lat: "-23.553236007700000",
        lng: "-46.574249267600000",
    },
    {
        lat: "-23.552421900000000",
        lng: "-46.573419800000000",
    },
    {
        lat: "-23.552553176900000",
        lng: "-46.573181152300000",
    },
    {
        lat: "-23.552079300000000",
        lng: "-46.572764700000000",
    },
    {
        lat: "-23.552452087400000",
        lng: "-46.573001861600000",
    },
    {
        lat: "-23.552553176900000",
        lng: "-46.573181152300000",
    },
    {
        lat: "-23.552080154400000",
        lng: "-46.572765350300000",
    },
    {
        lat: "-23.552339553800000",
        lng: "-46.571506500200000",
    },
    {
        lat: "-23.552373800000000",
        lng: "-46.571509900000000",
    },
    {
        lat: "-23.551967620800000",
        lng: "-46.571239471400000",
    },
    {
        lat: "-23.553024292000000",
        lng: "-46.572566986100000",
    },
    {
        lat: "-23.553100585900000",
        lng: "-46.572799682600000",
    },
    {
        lat: "-23.553188324000000",
        lng: "-46.573322296100000",
    },
    {
        lat: "-23.553508758500000",
        lng: "-46.573505401600000",
    },
    {
        lat: "-23.552674900000000",
        lng: "-46.573789000000000",
    },
    {
        lat: "-23.552713600000000",
        lng: "-46.573731100000000",
    },
    {
        lat: "-23.553152200000000",
        lng: "-46.571818300000000",
    },
    {
        lat: "-23.553367614700000",
        lng: "-46.571437835700000",
    },
    {
        lat: "-23.553100585900000",
        lng: "-46.571701049800000",
    },
    {
        lat: "-23.553808212300000",
        lng: "-46.571708679200000",
    },
    {
        lat: "-23.553110600000000",
        lng: "-46.571771400000000",
    },
    {
        lat: "-23.553100585900000",
        lng: "-46.571701049800000",
    }
];

@wf9a5m75
Copy link
Member

Hi everyone, this is Masashi who is the original author of this plugin.
I have already known this issue, many people crime to me since old versions, and I know what I should do.
I left this project at once (because it was crazy busy), but I came back to this project again.
Now, I have been fixing the issues currently filed one bye one.
I will buckle down this issue also. Hold on please.

@cvaliere
Copy link

Thank you @atannus for your work
Hi @wf9a5m75 it's good to have you back :)
Do you have an idea when you can release a new version?

@wf9a5m75
Copy link
Member

Sorry I have no idea when next is released at this point.

@atannus
Copy link
Author

atannus commented Mar 21, 2016

I'm happy to help with anything I can.

@wf9a5m75
Copy link
Member

wf9a5m75 commented Oct 5, 2016

Thank you @hirbod

@samazary
Copy link

samazary commented Oct 5, 2016

Great work @wf9a5m75

huge +1 for clustering support

I also just sent you USD100 - so you can have 50 more beer :)

@wf9a5m75
Copy link
Member

wf9a5m75 commented Oct 5, 2016

o-pint-glass-beer-facebook
@samazary Thank you

@cvaliere
Copy link

thank you @wf9a5m75, seems like a great upgrade
I can't use it yet because it crashes immediately with Xwalk. Are you working on it, or can you give me a tip of what should be done, and I'll try to do it?

@wf9a5m75
Copy link
Member

No xwalk tested. And some method and event names are changed. I recommened you use the plugin in a new project. Don't use it in your project immediately

@cvaliere
Copy link

ok, so, are you working on it, or you want someone else to work on it, and then can you give me a tip of what should be done? (I suppose you know what should be done, as the current master branch supports Xwalk)

@wf9a5m75
Copy link
Member

wf9a5m75 commented Oct 12, 2016

Basically I'm not interested in XWalk, but I tried a little.

I don't know why but mapView.onCreate(null); does not work with XWalk.
The below error is inside the MapView (I guess).

In that case, XWalk is not available.

Rejecting re-init on previously-failed class java.lang.Class<lz>: java.lang.NoClassDefFoundError: Failed resolution of: Lcom/google/android/chimera/Fragment;
    at java.lang.Class dalvik.system.DexFile.defineClassNative(java.lang.String, java.lang.ClassLoader, java.lang.Object, dalvik.system.DexFile) (DexFile.java:-2)
    at java.lang.Class dalvik.system.DexFile.defineClass(java.lang.String, java.lang.ClassLoader, java.lang.Object, dalvik.system.DexFile, java.util.List) (DexFile.java:299)
    at java.lang.Class dalvik.system.DexFile.loadClassBinaryName(java.lang.String, java.lang.ClassLoader, java.util.List) (DexFile.java:292)
    at java.lang.Class dalvik.system.DexPathList.findClass(java.lang.String, java.util.List) (DexPathList.java:418)
    at java.lang.Class dalvik.system.BaseDexClassLoader.findClass(java.lang.String) (BaseDexClassLoader.java:54)
    at java.lang.Class com.google.android.chimera.container.internal.DelegateLastPathClassLoader.loadClass(java.lang.String, boolean) (:com.google.android.gms:28)
    at java.lang.Class java.lang.ClassLoader.loadClass(java.lang.String) (ClassLoader.java:312)
    at maps.ad.as maps.ad.aw.a(android.content.Context, maps.y.J, maps.ad.ax, java.lang.String, boolean) ((null):0)
    at maps.ad.t maps.ad.t.a(com.google.android.gms.maps.GoogleMapOptions, boolean, maps.ad.c) ((null):0)
    at void maps.ad.R.a(android.os.Bundle) ((null):-1)
    at boolean wg.onTransact(int, android.os.Parcel, android.os.Parcel, int) (:com.google.android.gms.DynamiteModulesB:66)
    at boolean android.os.Binder.transact(int, android.os.Parcel, android.os.Parcel, int) (Binder.java:499)
    at void com.google.android.gms.maps.internal.IMapViewDelegate$zza$zza.onCreate(android.os.Bundle) ((null):-1)
    at void com.google.android.gms.maps.MapView$zza.onCreate(android.os.Bundle) ((null):-1)
    at void com.google.android.gms.dynamic.zza$3.zzb(com.google.android.gms.dynamic.LifecycleDelegate) ((null):-1)
    at void com.google.android.gms.dynamic.zza$1.zza(com.google.android.gms.dynamic.LifecycleDelegate) ((null):-1)
    at void com.google.android.gms.maps.MapView$zzb.zzbru() ((null):-1)
    at void com.google.android.gms.maps.MapView$zzb.zza(com.google.android.gms.dynamic.zzf) ((null):-1)
    at void com.google.android.gms.dynamic.zza.zza(android.os.Bundle, com.google.android.gms.dynamic.zza$zza) ((null):-1)
    at void com.google.android.gms.dynamic.zza.onCreate(android.os.Bundle) ((null):-1)
    at void com.google.android.gms.maps.MapView.onCreate(android.os.Bundle) ((null):-1)
    at void plugin.google.maps.PluginMap$1.run() (PluginMap.java:223)

@hirbod
Copy link
Contributor

hirbod commented Oct 12, 2016

Just giving my 2 cents on it: you might not be interested, but Cordova without Xwalk is a big piece of unusable shit. Xwalk=the same web view for every android device. The only real way to provide a good working app. So please don't ignore Xwalk

@wf9a5m75
Copy link
Member

wf9a5m75 commented Oct 12, 2016

Move your hands, please.
Although I'm not interested in XWalk, but I'm not leave it completely.
However I have no solution at this time.

So please move your hands to solve the issue.
Thanks.

@dsemerida
Copy link

dsemerida commented Oct 27, 2016

i have implement this code to render markers with the beta 2 map , according the result of this is on 1.0 secunds with the map beta 2 but on my project are 4.2 secunds ago, i dont idea, i have clone the beta 2 project and set cordova prepare but the time is the same

var map;
var mockData = [
  {
      lat: "-23.548922600000000",
      lng: "-46.574610800000000",
  },
  {
      lat: "-23.549337900000000",
      lng: "-46.574795600000000",
  },
  {
      lat: "-23.550828933700000",
      lng: "-46.576023101800000",
  },
  {
      lat: "-23.550889500000000",
      lng: "-46.576046300000000",
  },
  {
      lat: "-23.551199100000000",
      lng: "-46.575804400000000",
  },
  {
      lat: "-23.550761500000000",
      lng: "-46.575932700000000",
  },
  {
      lat: "-23.550820100000000",
      lng: "-46.575986100000000",
  },
  {
      lat: "-23.550958200000000",
      lng: "-46.576498800000000",
  },
  {
      lat: "-23.550552368200000",
      lng: "-46.574832916300000",
  },
  {
      lat: "-23.550529480000000",
      lng: "-46.575012207000000",
  },
  {
      lat: "-23.549200058000000",
      lng: "-46.573200225800000",
  },
  {
      lat: "-23.549949646000000",
      lng: "-46.572673797600000",
  },
  {
      lat: "-23.550093900000000",
      lng: "-46.572460600000000",
  },
  {
      lat: "-23.550067000000000",
      lng: "-46.572427400000000",
  },
  {
      lat: "-23.549314300000000",
      lng: "-46.571722400000000",
  },
  {
      lat: "-23.548916800000000",
      lng: "-46.572370800000000",
  },
  {
      lat: "-23.550040200000000",
      lng: "-46.571646100000000",
  },
  {
      lat: "-23.551219500000000",
      lng: "-46.571387300000000",
  },
  {
      lat: "-23.550374984700000",
      lng: "-46.571079254200000",
  },
  {
      lat: "-23.551379900000000",
      lng: "-46.572379600000000",
  },
  {
      lat: "-23.551279068000000",
      lng: "-46.571372985800000",
  },
  {
      lat: "-23.550819700000000",
      lng: "-46.571475300000000",
  },
  {
      lat: "-23.550776800000000",
      lng: "-46.571910200000000",
  },
  {
      lat: "-23.551599502600000",
      lng: "-46.575599670400000",
  },
  {
      lat: "-23.551599502600000",
      lng: "-46.575599670400000",
  },
  {
      lat: "-23.552324295000000",
      lng: "-46.575397491500000",
  },
  {
      lat: "-23.551766600000000",
      lng: "-46.575586100000000",
  },
  {
      lat: "-23.551599502600000",
      lng: "-46.575599670400000",
  },
  {
      lat: "-23.552499771100000",
      lng: "-46.574798584000000",
  },
  {
      lat: "-23.552565800000000",
      lng: "-46.574789700000000",
  },
  {
      lat: "-23.552565800000000",
      lng: "-46.574789700000000",
  },
  {
      lat: "-23.551799774200000",
      lng: "-46.573898315400000",
  },
  {
      lat: "-23.551788330100000",
      lng: "-46.574462890600000",
  },
  {
      lat: "-23.551799774200000",
      lng: "-46.573898315400000",
  },
  {
      lat: "-23.553100585900000",
      lng: "-46.575500488300000",
  },
  {
      lat: "-23.553400039700000",
      lng: "-46.575199127200000",
  },
  {
      lat: "-23.553100500000000",
      lng: "-46.574628900000000",
  },
  {
      lat: "-23.552700042700000",
      lng: "-46.573799133300000",
  },
  {
      lat: "-23.553236007700000",
      lng: "-46.574249267600000",
  },
  {
      lat: "-23.553236007700000",
      lng: "-46.574249267600000",
  },
  {
      lat: "-23.552700042700000",
      lng: "-46.573799133300000",
  },
  {
      lat: "-23.553236007700000",
      lng: "-46.574249267600000",
  },
  {
      lat: "-23.552421900000000",
      lng: "-46.573419800000000",
  },
  {
      lat: "-23.552553176900000",
      lng: "-46.573181152300000",
  },
  {
      lat: "-23.552079300000000",
      lng: "-46.572764700000000",
  },
  {
      lat: "-23.552452087400000",
      lng: "-46.573001861600000",
  },
  {
      lat: "-23.552553176900000",
      lng: "-46.573181152300000",
  },
  {
      lat: "-23.552080154400000",
      lng: "-46.572765350300000",
  },
  {
      lat: "-23.552339553800000",
      lng: "-46.571506500200000",
  },
  {
      lat: "-23.552373800000000",
      lng: "-46.571509900000000",
  },
  {
      lat: "-23.551967620800000",
      lng: "-46.571239471400000",
  },
  {
      lat: "-23.553024292000000",
      lng: "-46.572566986100000",
  },
  {
      lat: "-23.553100585900000",
      lng: "-46.572799682600000",
  },
  {
      lat: "-23.553188324000000",
      lng: "-46.573322296100000",
  },
  {
      lat: "-23.553508758500000",
      lng: "-46.573505401600000",
  },
  {
      lat: "-23.552674900000000",
      lng: "-46.573789000000000",
  },
  {
      lat: "-23.552713600000000",
      lng: "-46.573731100000000",
  },
  {
      lat: "-23.553152200000000",
      lng: "-46.571818300000000",
  },
  {
      lat: "-23.553367614700000",
      lng: "-46.571437835700000",
  },
  {
      lat: "-23.553100585900000",
      lng: "-46.571701049800000",
  },
  {
      lat: "-23.553808212300000",
      lng: "-46.571708679200000",
  },
  {
      lat: "-23.553110600000000",
      lng: "-46.571771400000000",
  },
  {
      lat: "-23.553100585900000",
      lng: "-46.571701049800000",
  }
];
document.addEventListener("deviceready", function() {
  var div = document.getElementById("map_canvas");

  // Initialize the map view
  map = plugin.google.maps.Map.getMap(div);

  // Wait until the map is ready status.
  map.addEventListener(plugin.google.maps.event.MAP_READY, function() {
    map.moveCamera({
      target : mockData
    });

    var start = Date.now();
    addMarkers(map, mockData, function(markers) {
      var end = Date.now();
      alert("duration: " + ((end - start) / 1000).toFixed(1) + " seconds");
    });
  });
  function addMarkers(map, data, callback) {
    var markers = [];
    function onMarkerAdded(marker) {
      markers.push(marker);
      if (markers.length === data.length) {
        callback(markers);
      }
    }
    data.forEach(function(position) {
      map.addMarker({
        position: position,
        icon: {
          url: "http://googlemaps.googlermania.com/img/google-marker-big.png",
          'size': {
            'width': 30,
            'height': 52
          }
        }

      }, onMarkerAdded);
    });
  }
});

@wf9a5m75
Copy link
Member

I have no idea.

@guillenotfound
Copy link

As response to @dsemerida :
Point 5 -> https://github.com/mapsplugin/v2.0-demo/issues/6#issuecomment-253334483

Also noticed this, but I didn't have so much time to deeply investigate it, I'd like to create a sample project with +300 markers and measure the time between 1.4 and 2.0

@alexbonhomme
Copy link
Contributor

Hey!

First, thanks for your job, this plugin is awesome :-)

I used the v1 (with Ionic 1) on my app and I'm currently writing a new version using Ionic 2 and the v2 of the plugin. Everything is fine except the markers creation. I add 222 markers on my map.
The v1 handle it without problem, but the v2 is extremely slow on Android :-(

~1.5s on this project (v1) : https://github.com/alexetmanon/vliller
~10s on this project (v2) : https://github.com/alexetmanon/vliller2

I make a test repo here : https://github.com/blckshrk/addmarker-debug
On my Nexus 5 it takes ~1.5s to add 100 markers and ~5s with 200 markers.

I have no idea from where it can come from... Should it be the Android SDK ? Is 200 markers a lot (it seems not on the v1) ? I've tried every thing, but really, I don't get it.

Could you take a look ? :-)

@wf9a5m75
Copy link
Member

@BlckShrk Specify the same icon as much as possible.
The v2 creates image cache internally when the icon properties (url and size) are the same.

@guillenotfound
Copy link

I also asked you about this when you released v2 @wf9a5m75 , there's something causing slow drawing of markers on the map in v2.

Point 5: https://github.com/mapsplugin/v2.0-demo/issues/6#issuecomment-253334483

But your answer about that point didn't help so much... https://github.com/mapsplugin/v2.0-demo/issues/6#issuecomment-253339084

I'd love to use v2 since I believe that it will work much better than v1, but adding lots of markers it's a common task, that HUGE time impact will be bad for UX...

@alexbonhomme
Copy link
Contributor

alexbonhomme commented Jan 27, 2017

Hey guys!

@wf9a5m75 I used the default icon for my tests (so no icons properties) and on the initial app each marker have the same icon object (with same url/size properties)

@ZiFFeL1992 I agree, adding more than 100 marker have a really huge impact on performances, but it seems limited to the Android version (to be confirmed)

@alexbonhomme
Copy link
Contributor

Hi,

I've taken a day to dig the problem and I found some issues.

Observations

  • The marker creation in the Android code (Marker.create) is fast (around 1ms by marker)
  • The time between the call of callbackContext.success(result); and the execution of the JS callback is really long (around 1,5s).
  • The plugin seems to use a lot of memory

capture d ecran 2017-02-07 a 09 18 49

capture d ecran 2017-02-07 a 09 24 10

Interpretation

I suspect a memory leak in the ResizeTask (and maybe somewhere else), which lefts no memory for the WebView. So the WebView needs to wait for the CG to dequeue the waiting callbacks.
I have a Nexus 5, so that could explain why I'm facing this problem and not you (less RAM). That also makes sense with the exponential evolution of the creation time according to the markers number.

Conclusion

I think we need to reduce the memory usage and fix the memory leaks to solve this problem. My advice is to avoid new as possible and maybe use iterators.

What do you think about ?

@wf9a5m75
Copy link
Member

wf9a5m75 commented Feb 7, 2017

@BlckShrk If you have find any problem, please fix it, and send it as a pull request with test code.

@bmcbride
Copy link

I'm also experiencing poor performance on Android with many (~400) markers in v2. Switching from v2 to v1 cuts loading time from 19 seconds to under 2 seconds. Has there been any progress on resolving this? I'm happy to help test, but I'm pretty sure resolving it is beyond my skill. Thanks!

@alexbonhomme
Copy link
Contributor

@bmcbride As described above, I observed the same problem, but can't found a solution...

@sam2x
Copy link

sam2x commented Apr 27, 2017

Hey i bring my own experience in this thread. I also have some issue with v2 when i use custom marker.
Here the contexts & tests:

Environment:

Cordova: 6.5.0
Cordova-plugin-googlemaps: 2.0.0-beta 
Android platform SDK : 6.1.2
Google Map SDK Google.android.gms:play-services-maps: 10.2.1
Phone test: Galaxy A5 

Application Context

Angular 1.6.4 using cordova, with 2views. The home view, and the Map view. Apk size is around 10Mo.

Test performed

I just switch between Home view and Map view. At each switch i check memory.

Test1: not custom marker

Test1 has 27 visibles and 39 invisibles markers during the whole test.

  1. Home: ~ 110 Mo (cordova + angular is known to have an important memory footprint, so no surprise here)
  2. Map: vary 175-185 Mo : not moving the map
  3. Home: ~ 160Mo (waited 30sec in case GC need some time)
  4. Map: vary 175-190 Mo : not moving the map (almost same as step2)
  5. Home: ~ 178Mo (waited 30sec in case GC need some time)
    ~ 162Mo (after 1min, almost same as step 3)

-> Looping between these two view seems to give almost the same results, which seems "legits" to me.

Test2: all markers are custom, size option is set to 32x32 (~10 differents png images, dimensions 36x36, between 1.5Ko and 2Ko each, and stored locally)

_Test2 has the same marker env as test1 => 27 visibles and 39 invisibles markers. Differents markers may set the same png. _

  1. Home: ~110 Mo
  2. Map: vary 175-185Mo (same result as previous test! seems good)
  3. Home: ~175-180Mo (hmmm not decreasing...)
  4. Map: vary 205-220 Mo (should be around the range value of step 2!? we got more than 20Mo here)
  5. Home: ~ 205 Mo (something is wrong)
  6. Map: vary 225-240 Mo (something is definitely wrong)
  7. Home: ~ 225 Mo (again ~20Mo memory seems to be retained somewhere).
  8. etc...

-> Looping between these two view increase memory, leading sooner or later the app to be killed by the system. So memory leaks likely occurs somewhere.

How i leave the Map view ?

I use destroy event in the Map view, in this function i do the following actions :

  • For each marker :

    • I use removeEventListener(plugin.google.maps.event.MARKER_CLICK) to clear the binded Click event;
    • I use remove() method of the marker;
    • I set to null the corresponding entry in the scope view Array that reference the marker (even if the scope should be automatically removed by Angular when the view is left) ;
  • For the map :

    • I use the clear() method ;
    • I use the off() method;
    • And at the end, i use remove() method to destroy the map;

Statements/Questions

Imho there is two distincts "statements":

  1. No matter the tests, there is ~60Mo that are persistents when we load the map, and even after destroying the map they still remains. (see steps 1, 2, 3)
  2. Memory allocation seems to not be released when creating new custom markers and destroying them (see tests 2).

So :

  1. Is this the GoogleMapService module that get loaded/mapped in memory (due to lazy loading ?) ? It would be normal behaviour so (isnt this kind of library shared accross all app in the phone and shouldnt appear as "consumed memory" in my app memory profil ? Not sure...).
    1'. Or some kind of cached data or others stuff not released/freed ? Any ideas ? Do you have the same behaviour ? Well not really my priority, but if you know, i'll appreciate.
  2. Markers: I think there is memory allocation that is not freed when using custom marker image. So the ressources is not cleaned despite my destroy step. I'm not sure where/how, have to look in the this plugin code (i'm not java reader, but i'll give a try).

Anyway, any pointers/ideas from your point of view ?

Regards

@alexbonhomme
Copy link
Contributor

@sam2x Imho, this is because the memory leaks are in the native code. So, destroy JS listeners won't clear native memory (I may said something wrong, I haven't looked at the remove() method)

Additionally, it seems that during the markers creation step some leaks appears (may some references aren't freed somewhere en lost after, waiting for a garbage collection for example).

Best,
Alex

@sam2x
Copy link

sam2x commented Apr 27, 2017

@BlckShrk thanks you for this notice. Yes, i'm aware this could be the native code guilty. However I'm trying first to take a deep look at the plugin (even if i'm not Java expert). Also It looks odd to me that so much memory is leaked from native code, in such simple "test-case", someone would have noticed if it was from native code (also i'm using last google-map sdk) ?

I'm investigating the way this plugin is dealing with BitMap (since it looks like a good culprit to me):

According the Android guideline Bitmap can "very easily exhaust an app's memory budget". They recommend to use the Glide library to "fetch, decode, and display bitmaps".

Looking at the code, i dont understand something. When creating a marker, with icon option specified, the setIcon method is called. Since we specified a size, this method call an util method resizeBitmap.
However in this resizeBitmap function we dont call "bitmap.recycle()" like scaleBitmapForDevice is doing.

@wf9a5m75 Isnt "bitmap.recycle()" missing here ? According my understanding from http://stackoverflow.com/questions/3823799/android-bitmap-recycle-how-does-it-work when you assign a new image, if you dont explicitely call "recycle()" the old one is still in memory.

I tried to rebuild my app without specifying the size (and let the plugin computes it via scaleBitmapForDevice which call recycle()) but result is still same as test2, so it seems to not solve the prob :/

So:

  1. i'm still investigating to see if there isnt any forgotten reference to Bitmap that need to be cleaned.
  2. Any reason why Glide is not used ? Will there be any benefit using "Glide" in this plugin ?
  3. The Bitmap cache system is only used for remote ressource (see above the description Performance problems when adding many markers. #835 (comment)). Why not also use it for local ressource ? It should load faster marker that use the same bitmap ?

@wf9a5m75
Copy link
Member

@sam2x Before asking easily, please try to debug (such as insert bitmap.recycle()) at first.

@wf9a5m75
Copy link
Member

@sam2x ok, could you make another issue at once? This thread is too long.
And please make an example project that reproduce your memory issue 100%, then share it on github or bitbucket.

@sam2x
Copy link

sam2x commented Apr 27, 2017

Is there a way to dicuss in PM about this ? i'm willing to financially contribute for your free time with this issue and make the effort/result available for the community. I know we all have priorities, and can't afford time easily.

@wf9a5m75
Copy link
Member

Please send email to me (see my github profile page).

@wf9a5m75
Copy link
Member

Since this thread is too long, I lock this thread.
If you have any problem with v2, please make another issue at the issue list page.

@mapsplugin mapsplugin locked and limited conversation to collaborators Apr 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests