googlemaps / android-maps-utils

Maps SDK for Android Utility Library
https://developers.google.com/maps/documentation/android-sdk/utility
Apache License 2.0
3.54k stars 1.53k forks source link

Unusable performance with >1k items in ClusterManager #1199

Open mbilalhussain96 opened 1 year ago

mbilalhussain96 commented 1 year ago

If I add over 1k items to the ClusterManager, the actual (drawn) markers are updated after very long delays. For example, I can have one cluster with "1000+" text on it, and when I zoom in, it stays for about 1-2 minutes, and only after that it splits into smaller clusters and markers.

  private lateinit var mMap: GoogleMap, 
   private lateinit var clusterManager: ClusterManager<mapclass>

  private fun addItems() {

        // welcomedataItemList1=1000

        for (place in 0 until welcomedataItemList1.size) {

            if (welcomedataItemList1[place]?.lat != null && welcomedataItemList1[place]?.lon != null) {

                var latitude =welcomedataItemList1[place].lat.toDouble()
                var longitude =welcomedataItemList1[place].lon.toDouble()
                serial = welcomedataItemList1[place].serial
                ownerName = welcomedataItemList1[place].ownerName

                status= welcomedataItemList1[place].adminStatus.toInt()
                mMap.moveCamera(CameraUpdateFactory.newLatLngZoom(LatLng(latitude, longitude), 6f))
                var offsetItem = mapclass(latitude, longitude, "$serial", "$ownerName")
                clusterManager.addItem(offsetItem)

            }
        }

    }

private fun setUpClusterer() {

        val metrics = DisplayMetrics()
        windowManager.defaultDisplay.getMetrics(metrics)

        clusterManager = ClusterManager(this, mMap)
        clusterManager.setAlgorithm(NonHierarchicalViewBasedAlgorithm(metrics.widthPixels, metrics.heightPixels))
        mMap.setOnCameraIdleListener(clusterManager)

        addItems()

        clusterManager.renderer = MarkerClusterRenderer(this, mMap, clusterManager)

        mMap.setInfoWindowAdapter(CustomInfoWindowForGoogleMap(this))

        mMap.setOnMarkerClickListener(clusterManager)
        mMap.uiSettings.isZoomControlsEnabled= true
        clusterManager.setAnimation(false);
        clusterManager.cluster()

    }

override fun onMapReady(googleMap: GoogleMap) {

      mMap = googleMap
            setUpClusterer()

        }
wangela commented 1 year ago

If you would like to upvote the priority of this issue, please comment below or react with :+1: so we can see what is popular when we triage.

@mbilalhussain96 Thank you for opening this issue. 🙏 Please check out these other resources that might help you get to a resolution in the meantime:

This is an automated message, feel free to ignore.

wangela commented 10 months ago

A suggestion from @cwsiteplan in the Compose issue discussion:

would it be possible to expand clusters only for the visible viewport? - seems like it's currently expanding on zoom level - adding lot's of individual markers on areas that are potentially not viewed at all

ColtonIdle commented 9 months ago

is this fixed with https://github.com/googlemaps/android-maps-compose/pull/421 ?

kikoso commented 9 months ago

@ColtonIdle , it is partially solved. Compose has an intrinsic lack of performance due to some limitations in the framework (i,e,, it has a penalty by using certain classes, and one of them is LatLng, which we use massively in a Cluster). So there is always going to be a level of lack of performance that we can't reduce (we are actually exploring this using a different mechanism, but that is another story).

This PR allows to choose another algorithm for the rendering. Namely, NonHierarchicalViewBasedAlgorithm only renders contain in the screen, which improves performance by a factor.

I have added a sample to the repository, for more clarity:

https://github.com/googlemaps/android-maps-compose/pull/463

Feel free to check it out.

ColtonIdle commented 9 months ago

Hm. so if we care about performance. we shouldn't use compose for clustering?

kikoso commented 9 months ago

At the moment, there is a certain level of intrinsic lack of performance in Compose. Again, we are exploring this (specifically, a new feature from the Compose 1.5.4 compiler called strong skipping).

Unless you are trying to cluster thousands of them, Compose will likely work (as long as you are also using the NonHierarchicalViewBasedAlgorithm)

ColtonIdle commented 9 months ago

Yeah, I currently have an app where we have about 16k items (mostly US and europe) hence my curiosity because we're not (yet) using compose maps. but we are using a fully compose android app with the exception of the map =)

im really eager to ditch AndroidView though as I've had a bunch of issues with clusters not recomposing properly. thanks for the update. and strong skipping seems cool. i wonder if we can just mark latLng as @Stable though?

kikoso commented 9 months ago

I would suggest you to check the Clustering with NonHierarchicalViewBasedAlgorithm, check out the PR I linked above. You can manually add another few thousands items, and see how the app behaves.

ShikaSD commented 9 months ago

Hi, one of the Compose devs here :)

Could you share a bit more about performance problems you have encountered? I am not personally familiar with Compose integration for Maps, but I'd be happy to chat about it and help debug / address perf issues with it.

kikoso commented 9 months ago

Hi @ShikaSD ,

The issue is happening in android-maps-compose.

It is likely related to this issue here, with the LatLng type not being inferred as stable.

Setting the stability via a config file and using strong skipping does not seem to fully remove the issue.

ShikaSD commented 9 months ago

@kikoso stability by itself does not result in performance problems that often, I doubt this is the cause here. I suspect the problem is in heavy content used for clusters. I'll look into attached issue a bit more.

ShikaSD commented 9 months ago

I ran a quick profile on the clustering sample with 1000 items. I think the new strategy that changes the viewport should help quite a lot, as it creates less items in general.

For Compose clusters, majority of the time is spent in creating and setting up AndroidComposeView instances to draw them on canvas. This is known to be slighly expensive as we need to setup wrappers for composition locals and other things, and certainly something we should optimize on our side as well. I also cannot help but wonder if there are better ways of rendering part of composition into a bitmap without involving a view per item, probably something worth optimizing for us well.

wangela commented 9 months ago

Can anyone verify that using NonHierarchicalViewBasedAlgorithm resolves your high-volume clustering performance issues? @mbilalhussain96 I see in your original post that you were already specifying NonHierarchicalViewBasedAlgorithm, but your specifications for width and height differ from the demo app which specifies metrics.widthPixels / metrics.density for width and metrics.heightPixels / metrics.density for height.

cc @ColtonIdle

kikoso commented 9 months ago

@mbilalhussain96 , could you eventually share your implementation of MarkerClusterRenderer?

ColtonIdle commented 8 months ago

For anyone that was curious of how to grab height and width from compose code, you can see a discussion I brought up in kotlinlang slack: https://slack-chats.kotlinlang.org/t/16243087/what-would-be-the-compose-way-of-grabbing-these-window-value#eb72b9b4-d4a9-462d-bbd7-c727dfed29d3

LouisCAD commented 8 months ago

TL;DR is use Modifier.onSizeChanged.