Proposal: Prevent unnecessary re-renders

[fgnass]

trafficstars

On maps with only a small amount of clusters, it often happens that all clusters are redrawn upon zoom, even though nothing has changed. The resulting flickering looks very buggy and distracting. As a workaround, we use an algorithm that compares the actual markers:

import {
  AlgorithmInput,
  Cluster,
  SuperClusterAlgorithm,
  SuperClusterOptions,
} from "@googlemaps/markerclusterer";
import { shallowEqual } from "fast-equals";

const summarize = (c: Cluster) => `${c.position} ${c.count}`;

export class CachedSuperClusterAlgorithm extends SuperClusterAlgorithm {
  constructor(opts: SuperClusterOptions = {}) {
    super(opts);
  }
  calculate(input: AlgorithmInput) {
    const prevClusters = this.clusters;
    const output = super.calculate(input);
    const { changed, clusters } = output;
    if (changed && prevClusters) {
      if (shallowEqual(prevClusters.map(summarize), clusters.map(summarize))) {
        return { changed: false, clusters };
      }
    }
    return output;
  }
}

I was wondering if it made sense to incorporate such a check into the library, and would be happy to contribute a PR if you think this would be a good idea.

[jpoehnelt]

@fgnass Thank you for opening this issue. 🙏 Please check out these other resources that might be applicable:

• Support Console - If you have a support contract.
• Discord - chat with other developers
• StackOverflow - use the google-maps tag
• Google Maps Platform issue trackers

This is an automated message, feel free to ignore.

[jpoehnelt]

Seems reasonable to add this. The flickering is definitely suboptimal. What use cases exist for not doing this?

[fgnass]

I can't think of any, besides of performance, maybe. I'll create a PR and change the code so that the string-conversion only happens when the size of the arrays is equal, and also comparing the items one-by-one instead of mapping both arrays upfront.

[fgnass]

While implementing my proposed solution, I realized that caching on algorithm level is not a good idea, since the renderer might still want to display the markers differently based on the ClusterStats which are likely to change, even if the position and count of an individual cluster stays the same.

I therefore moved the caching to markercluster.ts which now takes the output of the renderer into account

Here's a screen capture of the improved version:

https://user-images.githubusercontent.com/89450/156614421-e419df06-b7e6-4142-9bc4-28614cdc5560.mov

.

[fgnass]

For reference, here's how the same example looks with the original implementation:

https://user-images.githubusercontent.com/89450/156614658-237bb46a-be5a-463b-980e-ed19e9e026d7.mov

:

[fgnass]

Even though it's already much better, I'm still not super happy with the result. When looking at the performance panel in the dev tools, there are still a lot of dropped frames and tasks that take much too long.

One path of optimization that might be worth to test, would be to reduce the load on the garbage collector by reusing the cluster markers instead of throwing them away.

If I find some more time, I will give it a try. If you want, you can take a look at the current work in progress in my forked repo.

Another interesting find was, that the actual markers flicker a lot less, if they are constructed with {optimized: false}.

[fgnass]

I found some time to try out a few more ideas. The one that provides the best results does the following:

• After each render pass, remember the markers that have been rendered
• Then, in the next render pass:
  • For each marker, look up the closest one from the previous pass
  • If it was not more than 45px away, reuse and update it

In order for this to work, renderers must return google.maps.MarkerOptions. If they return Marker instances like before, the algorithm falls back to the old behaviour.

I did some additional profiling to check where most of the CPU cycles are spent and noticed that the number of DOM nodes that are needed for a single marker has quite an impact. I updated the default renderer so that the label is now part of the SVG icon, which saves a lot of nodes.

Here's a screen capture that shows how markers now "morph" into their new state:

https://user-images.githubusercontent.com/89450/156791186-ac92e31c-d8b0-4812-b1ad-41f7fd078fbb.mov

[bdcoder2]

Just my two cents worth -- this "problem" will likely persist as the original clusterers design was to use Marker objects as input. A better approach is to use "data-points" that contain a latitude, longitude and data key. Using the data-points approach saves on memory and CPU -- especially when it comes time to render markers on the map as only those markers in the visible area of the map need to be created. I have tested this and found a data-point design to be significantly more efficient, and without any "flickering" !

Code is available for play at: https://github.com/bdcoder2/grid_clusterer

[fgnass]

@bdcoder2 I checked out your example, but unfortunately it exposes the same kind of flickering as the original (at least on my machine). If you play back the screen recording frame by frame, you can see that all markers briefly disappear before the new ones are rendered.

https://user-images.githubusercontent.com/89450/156990458-1bac667f-4484-4910-a458-3c553ed4980a.mov

In my experience, using markers as input is only a problem if they are initially added to the map. Unless they are added to the DOM, their overhead is pretty much negligible.

[bdcoder2]

@fgnass - Yes, by design, the grid clusterer has to remove (the smaller) grid points when cluster markers are built. The same goes when a cluster marker is clicked, the cluster marker is removed and smaller grid-point markers are added, so it may seem like markers are flickering (on some machines). Not sure anything can be done as the marker rendering is handled by Google Maps. On my machine the markers appear smooth.

With regard to using markers as input -- if you run the first demo entitled "Creating data points vs creating Google map markers" - you will see up to a 90% improvement in speed. Memory savings are also significant -- as I have run the grid clusterer with up to 6 million data points without issue, if I try to create 6 million map Markers, my browser would consistently become unresponsive as others have posted.

[fgnass]

@bdcoder2 I totally get your point, but this issue is not about the initial load or performance improvements in general, but solely about trying to reduce the number of markers that need to be added/removed on each render cycle. All of this without introducing any breaking changes.

@jpoehnelt I'm quite happy now with how the result turned out and would appreciate your feedback. Of course, the docs would need to be updated too, in order to mention that custom renderers should return MarkerOptions instead of Markers to benefit from the performance gains. Please let me know if you would be interested in a PR.

[bdcoder2]

@fgnass - No problem - your modifications will result in a better visual experience and will save some CPU cycles. However, using map markers internally will always be more expensive (due to the memory requirements of each map marker object). Again, thanks for the improvements!

[wangela]

@fgnass I saw you mentioned that you're using svg for your markers; changing to png would also help performance. See this YouTube video and the links in the description for more detail. https://www.youtube.com/watch?v=rU9OcRrNkVg

[aron-hivebrite]

Just FYI @fgnass https://stackoverflow.com/questions/73838247/markers-and-clusters-are-flashed-redraw-when-zoom-is-changing-googlemaps-js-mar

[jeffschulzusc]

at https://github.com/googlemaps/js-markerclusterer/blob/1193ff4/src/markerclusterer.ts#L114

what is the purpose/usage (there is no explanation in the documentation) of: onAdd

it seems to bind to idle and render ... and this seems to be the cause of 'flashing' renders in my app ... for me the markers do not flash when i circumvent the onAdd via this.clusterer.onAdd = () => {};

and actually, i don't see where onAdd would be called from

[vicb]

onAdd is from the base class (OverlayView). See https://developers.google.com/maps/documentation/javascript/customoverlays#add