android-maps-utils icon indicating copy to clipboard operation
android-maps-utils copied to clipboard

Unusable performance with >1k items in ClusterManager

Open mbilalhussain96 opened this issue 1 year ago • 16 comments

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()
        
        }

mbilalhussain96 avatar Jun 30 '23 12:06 mbilalhussain96

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 avatar Jun 30 '23 12:06 wangela

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

wangela avatar Oct 31 '23 18:10 wangela

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

ColtonIdle avatar Nov 27 '23 07:11 ColtonIdle

@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.

kikoso avatar Nov 27 '23 08:11 kikoso

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

ColtonIdle avatar Nov 27 '23 08:11 ColtonIdle

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)

kikoso avatar Nov 27 '23 08:11 kikoso

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?

ColtonIdle avatar Nov 27 '23 08:11 ColtonIdle

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.

kikoso avatar Nov 27 '23 08:11 kikoso

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.

ShikaSD avatar Nov 28 '23 13:11 ShikaSD

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.

kikoso avatar Nov 28 '23 13:11 kikoso

@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 avatar Nov 28 '23 13:11 ShikaSD

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.

ShikaSD avatar Nov 30 '23 07:11 ShikaSD

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

wangela avatar Dec 15 '23 20:12 wangela

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

kikoso avatar Dec 15 '23 20:12 kikoso

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

ColtonIdle avatar Jan 09 '24 01:01 ColtonIdle

TL;DR is use Modifier.onSizeChanged.

LouisCAD avatar Jan 09 '24 12:01 LouisCAD