High cpu usage during mousemoves - unnecessary render calls?

[ambientlight]

mapbox-gl-js version: 0.44.1 mapbox-gl-draw version: 1.0.3

I have noticed generally high CPU usage on mousemove in simple_select mode which comes from DrawContext.store.render() from event delegate.

Why does render needs to be performed on mousemove in simple_select. Isn't its purpose is just to query features and update the cursor to indicate selection? Or am I missing something?

[mcwhittemore]

@ambientlight - Yea, if there is a good way to skip the render here we should do that. Looking I don't see a clear way to simple way to achieve this without adding some sort of internal "skipRender" function. If you have an idea, please feel free describe here or show it in a PR. Perf improvements are always welcome :).

[mcwhittemore]

@ambientlight - I see you did some work on this in a fork. While that commit would fix the problem for simple_select it wouldn't fix it for other modes such as direct_select or any custom none drawing mode.

[ambientlight]

@mcwhittemore: for me most critical was to just get rid of unnecessary re-renders in simple_select. Since it accounted for vast majority usage time in my particular case I went with this hotfix of just not letting delegate calls to go through. I haven't used it for other modes as I didn't have time to get into details of how mapbox-gl-draw works internally, so I have created this issue to make sure I am not doing anything wrong with such approach. I was a bit confused since I didn't understand why render needed to be called for 'simple-select' actually.

Maybe adding requiresRender flag to each mode instance and then having render() logic adjusted for it might be a reasonable way to approach this. Also I found that each geoJSONSource.setData() triggers renders so maybe memoizing query results may be another improvement that could be made to avoid unnecessary rerenders when let's say we are highlighting objects for the sake of selection.

[hyperknot]

I'm in the same boat. I actually don't want to use simple_select or direct_select at all, I manage selection myself, with an Edit button in the UI.

For this reason, I'm using the official example static mode, but even that one is triggering continuous render calls. How can I make a mode which doesn't do anything at all?

[mcwhittemore]

After working on #783, a solution here would be to add a render flag to mode event handler functions. If the flag equals false than store.render doesn't get called after the event handler runs. If the flag is undefined or true than it would get a render.

I'm pretty heads down on a project right now and will be for a bit so I don't have the mental space to validate, code, test and update the docs for this right now. If someone else could pick this up, I'm more than happy to do the review.

The non test part of this would be pretty easy, you swap https://github.com/mapbox/mapbox-gl-draw/blob/master/src/lib/mode_handler.js#L41 for:

if (handle.fn.render !== false) DrawContext.store.render();

[mikeomeara1]

We ran into this same problem and noticed that this problem gets much more pronounced when you're dealing with larger maps. The performance penalty seems directly related to the size of the map and the devices devicePixelRatio -- Which makes sense based on what is described above.

I wasn't able to get the above comment to work....I'm guessing there is more to this than just that one line? Sorry if we're being dense.

The only thing that mitigated this at all for us (admittedly housefly-with-a-hammer) though is what others are describing on this thread....disable mouse movement tracking for simple and direct select - which I have no idea what the side effects of are -- and this still is problematic for all other modes.

[hyperknot]

@mikeomeara1 for me the key was to only enable draw when the user actually draws, and immediately remove it when it's not needed. Otherwise the performance is critically slow.

Also, I replaced simple_select and direct_select with a custom modes so that it's at least usable.

[mikeomeara1]

@hyperknot Thanks so much for the suggestion...that's also the conclusion we're coming to over here. However, for our app, the workflow is Plot --> Draw --> Report so turning it off and on is really a deal-breaker for us. We're leaning heavily into Mapbox's ability to manage drawing state and is heavily used throughout all aspects of the app. Unfortunately for us.

If it helps you or anyone else, here are our findings of several days of probing:

When in direct/simple select, the biggest offender is this line: events.js#49

So, we found that bypassing that statement using something simular to what you came up with does the job but still allows vertex selection, shape movement and move-move tracking for all other modes (there is probably a more efficient/prettier way to do this...but it's good enough for a POC):

...
if(event.api.getMode() !== 'simple_select' && event.api.getMode() !== 'direct_select')
   currentMode.mousemove(event);
...

This is basically a different form of @hyperknot solution. This brings mouse move on our 1500px+ map from 40-90% down to 15-20%

Then we added what I've outlined here: #888 to manage to the unwieldy keydown handler that also contributes 30-40% CPU while any key held. However, I think that debouncing so heavily (500ms) is too much, so I'm still working at this one.

Those two fixes brought us down to something in the neighborhood of: mousemove: 15-25% CPU keydown: 1-15% mousemove + keydown: 15-40%

Not perfect but steps in the right direction. If it's useful, I'll continue posting my findings here.

Because our app is an Angular app, we also have a number of zone.js and hammer.js fixes that had to go in to eliminate other areas of CPU hogging-ness. If anyone is interested in our findings there, just let me know.