visgl / react-google-maps

React components and hooks for the Google Maps JavaScript API
https://visgl.github.io/react-google-maps/
MIT License
1.19k stars 92 forks source link

[Bug] Rendering many Advanced Markers as map children is extremely slow #171

Open sspread opened 8 months ago

sspread commented 8 months ago

Description

On initial component render, even a few dozen Advanced Markers as map children will take ~ 10 seconds to initially render map. A few hundred can crash the browser.

Interestingly, a workaround is to render 1 marker, then set timeout for > 100ms and render many. OR use legacy Marker component.

Steps to Reproduce

Render 100 AdvancedMarkers as map children (just with key, position props)

Environment

Logs

No response

sspread commented 8 months ago

Found another workaround, so issue must have something to do with the library loading. Hold the rendering of my component until marker library is loaded.

const Loaded = ({ children }) => {
  const markers = useMapsLibrary('marker')
  const isLoaded = useApiIsLoaded()
  return isLoaded && markers && children
}
usefulthink commented 8 months ago

That's very helpful, thanks. I think I have an Idea what might be going wrong there...

usefulthink commented 7 months ago

I tried to somehow reproduce this issue, but even with 1000 advanced markers i can't see any problems. Even in the timeline recording I can only see that creating 1000 markers and adding them to the map takes about 190ms.

sspread commented 7 months ago

Are your markers rendered in the first map render? i.e. not added afterwards.

usefulthink commented 7 months ago

yep, this is the code i was testing with.

const App = () => {
  return (
    <APIProvider apiKey={API_KEY}>
      <Map
        mapId={'bf51a910020fa25a'}
        zoom={3}
        center={{lat: 12, lng: 0}}
        gestureHandling={'greedy'}
        disableDefaultUI={true}>
        {Object.keys(Array.from({length: 1000}))
          .map(Number)
          .map(i => (
            <AdvancedMarker
              key={i}
              position={{
                lat: 20 - Math.floor(i / 50) * 4,
                lng: -100 + 4 * (i % 50)
              }}></AdvancedMarker>
          ))}
      </Map>

      <ControlPanel />
    </APIProvider>
  );
};
usefulthink commented 7 months ago

This is what it looks like in the profiler. The highlighted block is creation of the 1000 marker-elements and adding them to the map, nothing suspicious there... image

sspread commented 7 months ago

Tried with your test code and its crashing Chrome unless I lower count to ~100 (so same issue for me)

usefulthink commented 7 months ago

Ok, that's really weird. What kind of GPU do you have? Anything different when opening it in an incognito-tab? Or maybe chrome canary? Tried all the options I have and it works every time.

sspread commented 7 months ago

When its slow, its re-rendering my component hundreds of times before painting. So, if I added a console.log('render') in your test 'App' component, it would log a lot. I don't think it has anything to do with chrome or gpu .

fc-jeremy commented 7 months ago

I am having this same issue with over 100 advanced markers

Edit: can confirm the Loaded workaround above fixes it for me.

luis-cicada commented 6 months ago

Im not sure if this is the same issue, but I was having issues after rending clusters and trying to play around with the map. The map was presenting a slow performance. This was not happening on loading but after the cluster rendering.

The solution:

using useMemo for all the cluster improve the performance A LOT!!

const markerCluster = useMemo(() => {
    return (
      <>
        {data.map((point, index) => (
          <AdvancedMarker
              position={{ lat: point.latitude, lng: point.longitude }}
              ref={(marker) => setMarkerRef(marker, index.toString())}
            ></AdvancedMarker>
        ))}
      </>
    )
  }, [markers])

  return <>{markerCluster}</>
usefulthink commented 6 months ago

@luis-cicada in that case you're also missing the key-prop for the AdvancedMarker components, so react will recreate all markers with every change. Can you try just adding the key-prop and remove the useMemo?

usefulthink commented 6 months ago

@sspread @fc-jeremy I've been thinking about this and how it could be that me and my colleagues are unable to reproduce this. Do you have a way to provide an URL to something where you see the issue?

Which bundler and settings (babel-presets?) are you using? I was thinking that maybe this could be caused by not using native async/await in the bundled code. That would at least somewhat explain why just waiting once for the loaded-promise seems to resolve this issue.

luis-cicada commented 6 months ago

@usefulthink I do have key-prop but Im using a custom component and even with key-prop it was extremly slow

<>
      {data.map((point, index) => (
        <BaseMarker key={index} index={index} point={point} setMarkerRef={setMarkerRef} />
      ))}
</>
bostrom commented 6 months ago

I experienced the same slowness when using the marker clustering example from the docs out of the box and swapping out markers dynamically.

Just rendering the AdvancedMarker without any clustering works fairly fine even with a few thousand markers, but adding the Markers clustering component makes the app crash with a "call stack too deep" error when the marker data changes.

usefulthink commented 6 months ago

@bostrom interesting observation, but I think this is a separate issue, I gotta have a look into the marker-clustering at some point..

andreasremdt commented 4 months ago

I have a similar observation. Initially, rendering markers is okay-ish in terms of performance, but after swapping markers for a while it's quickly getting slower and slower. And I am talking about 10 markers top, not 100 or 1000. After like 10 swaps, it takes ~5 seconds to render and the entire app becomes unusable.

cuongdtt commented 3 months ago

Confirm @sspread workaround works for me with >80 AdvancedMarker

RomanBobrovskiy commented 2 months ago

Any updates here? It's still very slow with >1000 of AdvancedMarker elements, though it's working faster with deprecated Marker element

usefulthink commented 2 months ago

Sorry, no updates from our side. There are so far no indications as to what could be the reason for this, and there hasn't been an example provided where we can see the problem occurring (I tried reproducing it myself, but I don't see any problems even with 1000 AdvancedMarkers).