-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Map is recreated too often #8823
Comments
See also #8087, which describes another issue with the map that impacts performance. |
@thomasneirynck I noticed this as well when working a different issue. I thought I would share what I found. The maps get recreated whenever an elastic search response is generated. Thus, sometimes when you zoom depending on auto precision setting and obviously when the data/options change. I then realized that the original map is executing its events (like zooming) and performing whatever actions it needs to first. Then once that map is done with its events we determine if we need to do a new elastic search. If a new search is requested thats when we recreate the map from that response while the old one just gets thrown away. We're waiting for a map to complete its event only to just throw it away later. Like you said it feels sluggish. You feel as though the map isn't keeping up with you. You zoom and see the map change, then maybe try something else on the map but its now getting removed from the DOM because the event caused us to receive new aggregations and you're now interfacing with a different map with different markers. Zooming is the biggest culprit of this because the markers change size more than once. First on the map zoom with the original marker set and then with the new set of markers from the new aggregation response. I'm wondering if the maps are being recreated because we know that it will need to create new markers, update the legend, etc. because of the new aggregations it received. So we took the path of least resistance and just recreated them. With all that being said, it's hard to say if changing the code to update the existing map would make the new aggregation data appear that much faster on the map. It would still have to make lots of updates to the map (new legend, new markers, etc.). I think some visual changes could help with the user experience in zooming that causes new aggregations needed to be displayed. Here are some options maybe:
I'm curious what you and others think about how to fix this issue? As far as performance hits though I think #8087 is the larger issue. |
All visualizations are destroyed and recreated when resized, new ES data, or the state has changed. I tried a fairly large change to convert the visualizations to use d3's object constancy (back when maps was part of the vislib) and it definitely made things snappier. But it was a larger change that I wasn't sure if kibana wanted and after the last few major modifications to vislib, it doesn't work anymore. I can try and resurrect it and submit it as a pull request for discussion if you are interested. It doesn't involve new files and such. It just changes the d3 code to have true enter/update/exits and adds a few more d3 parts where d3 wasn't being used. |
That's a fair assessment imho. As for performance in general, recreating the map on zoom hurts interactivity. It is not the kind of snappy zoom experience people expect (issues with scroll, pinch zoom). @trevan @JacobBrandt I'm in agreement, the map right now needs work, and we're on the way there. For scope of what's on the table, see #8944. As for introducing large PRs, at this moment that's somewhat difficult. Work on vislib and Visualize will likely continue for a while, putting that code in flux in the short term. |
@trevan Wow, that is really interesting. Kudos for getting it to work with d3's enter/update/exits. I can see where there would be major improvements in doing that. @thomasneirynck Yea I saw the Tilemap meta issue. I agree with you that this is a glaring issue, it's how I got here. 👍 . I think this along with 8087 like you say is a high priority to getting Tilemap where we all want it to be. So if we don't recreate the map from new ES data and instead just update it then the user at least will be interacting with the same map which I agree is a good thing. Although, I still think we won't have the snappy zoom experience people expect. I mean lets face it users want instant feedback. The issue with that is when you zoom and your aggregation features on your map need to be recreated because the zoom caused us to retrieve a new set of aggregations that's going to take time. So how should you present that time to the user? I think what we have right now might be confusing the user. Right now they get instant feedback from the tile map service through the zoom animation, and in effect see their layers change instantly as well. However, what they are missing is that what they are seeing is misleading. The aggregation features that are changing on the zoom are the old ones and they actually need to wait for the new aggregation data so that the map can display the real data at this new zoom level. What do people think of the two options I presented earlier? No matter what we do you have to wait for the new ES data, create features from it, and add them to the map which right now we don't visually let the user know about. I think this delay is best done upfront and not done after waiting for the zoom animation to finish. I would really like to help solve this one so any other ideas anyone has lets here them. |
the entire map is recreated when zooming. This includes any layers that it may have (e.g. the heatmap layer).
it is also recreated when there are changes in the data/options panel.
It is the main reason why the map feels sluggish.
The text was updated successfully, but these errors were encountered: