MarkerClustererPlus v2.1.1 (2013) Outperform MarkerClustererPlus Latest Version

[xtianpu]

Hi! I found out MarkerClustererPlus v2.1.1 (2013) Outperform MarkerClustererPlus Latest Version,

Check this out: https://jsfiddle.net/putrapurba/9Lhzujxg/48/

Looks like MarkerClustererPlus v2.1.1 rendering faster and doesn't freeze while Zooming In, Zooming Out, or Dragging the map.

Is it because current version MCP drawing and clustering at the same time? (I suspected this, because the label counter on cluster icon was increasing on load)

Thanks

[xtianpu]

@garylittleRLP

[timofei-iatsenko]

Well, I investigated this issue.

I compared source code of these two versions and found that issue comes from Cluster.addMarker method. Version from this repository hasthis.updateIcon_(); call at the end of the function.

The version from Cloudflare has no such call, updating cluster icons is done in the MarkerClusterer.createClusters_ method after firing clusteringend event.

The version hoisted https://cdnjs.cloudflare.com/ajax/libs/markerclustererplus/2.1.4/markerclusterer.js according to what is written here comes from repo mahnunchik/markerclustererplus which is, as following from readme, is a git clone of old google's SVN repo.

I dug commits in current git repo and tried to find who and when added this call to the method, but no luck. The very first commit which I was able to find already contained this call. I also checked commits tagged with version from 2.0.1 to 2.1.6 and there weren't any changes related to the issue.

Looks like either version in mahnunchik/markerclustererplus was edited and then pushed to Cloudflare, or it's copy of another version from google's SVN. Anyway, here we have some version misleading.

And this definitely a bug, clusters shouldn't be re-rendering on every batch processing. It should be rendered once when all markers clustered and cluster info is calculated.

BTW, I prepared a patch from changes which I used to prove my ideas on current repo. Here it is. I could prepare a PR, but i'm not sure why it was done like that, and what consequences might be, because there are no unit or e2e test.

Maybe somebody from code owners (@jpoehnelt) can shed light on this situation.

Index: packages/markerclustererplus/src/cluster.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- packages/markerclustererplus/src/cluster.ts (revision db69bf4a5c0af09809f6301440b6f07e0c1cdea3)
+++ packages/markerclustererplus/src/cluster.ts (date 1578915287752)
@@ -170,7 +170,6 @@
       marker.setMap(null);
     }

-    this.updateIcon_();
     return true;
   }

@@ -196,7 +195,7 @@
   /**
    * Updates the cluster icon.
    */
-  private updateIcon_(): void {
+  public updateIcon(): void {
     const mCount = this.markers_.length;
     const mz = this.markerClusterer_.getMaxZoom();

Index: packages/markerclustererplus/src/markerclusterer.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- packages/markerclustererplus/src/markerclusterer.ts (revision db69bf4a5c0af09809f6301440b6f07e0c1cdea3)
+++ packages/markerclustererplus/src/markerclusterer.ts (date 1578915287761)
@@ -1130,6 +1130,10 @@
     } else {
       delete this.timerRefStatic;
       google.maps.event.trigger(this, "clusteringend", this);
+
+      for (let i = 0; i < this.clusters_.length; i++) {
+        this.clusters_[i].updateIcon();
+      }
     }
   }

[xtianpu]

Ahh I see, nvr thought you gone that far to investigate. So, while this issue still waiting for @jpoehnelt or the author @garylittleRLP, I could fork and test current version based on your suggestion. thanks anw.

[jpoehnelt]

Maybe somebody from code owners (@jpoehnelt) can shed light on this situation.

I don't know the background on this. I would suggest sending a pr and if the samples work, let's merge and go from there.

[garylittleRLP]

The change mentioned above seems to be a good one. I've been using the unmodified version successfully for several years but this fix probably speeds things up a bit. Kudos to @thekip for his sleuthing.