Skip to content
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

Optimize GeoHash rendering #2119

Closed
robyngit opened this issue Apr 7, 2023 · 3 comments
Closed

Optimize GeoHash rendering #2119

robyngit opened this issue Apr 7, 2023 · 3 comments
Assignees
Milestone

Comments

@robyngit
Copy link
Member

robyngit commented Apr 7, 2023

On the feature-#1720-cesium-data-catalog branch, geohashes are rendering reasonably quickly now. However there are a couple of remaining issues:

  1. When the view is centred on the poles and very zoomed in, too many geohashes render and the map crashes. When the number of Geohashes in the collection exceeds some set amount, we should "down sample" and use the lower precision geohashes instead.
  2. Occasionally, after scrolling around the map for awhile, Cesium freezes and then crashes. This could be the same issue as 1 but I suspect something else is going on. Every time we update the geohashes on the map, we remove the old entities and replace them with new ones. If the old ones are not getting removed properly, maybe there is a memory leak?
@robyngit robyngit self-assigned this Apr 7, 2023
@robyngit robyngit added this to the 2.24.0 milestone Apr 7, 2023
robyngit added a commit that referenced this issue Apr 19, 2023
- Reduce redundant Solr requests
- Reduce geohash precision when there are too many geohashes to display
- Reduce geohash precision when the SpatialFilter query string is too long

Relates to #2119
@robyngit
Copy link
Member Author

I've improved the performance a little, but I'm still seeing problem num. 1 described above. It seems that faceting is either the cause of or a contributor to the map crashing when zoomed in at the poles.

robyngit added a commit that referenced this issue May 3, 2023
- Work around the issue where we request a high precision of geohashes that cover a very large area and cause the browser to crash
- Calculate precision from bounding box rather than height
- Also add other minor performance improvements to related methods
- Implement new methods in SpatialFilter model
- New methods not yet implemented in CesiumGeohash layer model

Issue: #2119
@robyngit
Copy link
Member Author

robyngit commented May 3, 2023

🎉 Big breakthrough on this issue to share! After encountering many dead ends, I finally found and fixed the source of the problem.

The issue was not related to faceting or how we were rendering geohashes. Instead, the problem is tied to trying to perform operations with too many geohashes — something we'd only encounter with a 3D map like Cesium and not with Google Maps.

When the camera is heavily tilted, we end up calculating a very low "height" or "altitude." Since we were deciding on a geohash precision level based on a simple mapping of height:precision, we were requesting a high precision of geohashes for these tilted views. The bounding box that we calculated in these views also ends up being large, since the distant background area is still in view. All of this meant that we needed hundreds of thousands of geohashes to cover the extent at the given precision. A similar problem occurs at the poles.

Even just obtaining the hashStrings from the nGeohash library using the bounding box and precision was sometimes extremely slow. Then, when we tried to consolidate these hashStrings by replacing complete groups of 32 geohashes with their parent (1 precision level lower), the browser would often crash.

To address this issue, I've made these changes, among others:

  1. Instead of mapping height/altitude in meters to geohash precision, we now calculate the ideal precision based on the bounding box and a maximum threshold of allowed geohashes.
  2. We also consolidate hashStrings immediately upon requesting them for a bounding box. We now iterate through the possible precision levels, starting with the least precise, until we find a mix of geohashes that cover the given area. See this part of the latest commit for details.

These changes have been implemented in the SpatialFilter model but not in the CesiumGeohashes model. With the CesiumGeohashes layer turned off, the spatial filtering is now much faster and stable. I've tested it with a variety of camera views, and I haven't been able to crash the map yet.

Here's what's left to do:

  • Adapt CesiumGeohashes to use the new methods, which will be slightly different since we don't want to display consolidated geohashes. I expect the improvements to still be similar.
  • Prevent redundant Solr queries. Currently, we send two requests to Solr whenever the camera placement changes—one from SpatialFilters and another from CesiumGeohashes (for facet counts). We can combine listeners in one of the connector models to send a single request with each camera change and then pass the results to the respective models.
  • Test all these changes thoroughly, a little bit of code clean up.
  • Update the demo and see if anyone on the team can still manage to crash the map.

Side note: I think it would cool if one day we could use high precision geohashes in the foreground, and low precision hashes in the background, getting less precise the further they are from camera. This would add a lot of complexity and isn't required for the next release, but it could be a nice feature in the future.

robyngit added a commit that referenced this issue May 4, 2023
Other minor fixes and improvements:
- Add configurable collection-wide min and max precision values in Geohashes (set max to 9 rather than 6)
- Always validate precisions and bounds in methods that use them in Geohashes collection
- Account for bounding boxes that cross the antimeridian or have north < south when calculating area
- Fix issue where the search results remained in "loading" state when the search was cancelled (e.g. the request URL didn't change, happens when the Cesium camera view has not changed sufficiently)
- Decrease limit of number of geohashes in spatial filter to respect Solr's boolean clauses limit
- Fix issue where pager was hidden when there were 25-50 results
- Remove old CesiumGeohashes view
- Remove unused methods from CesiumGeohash model & Geohashes collection

Issues: #2119, #1720
robyngit added a commit that referenced this issue May 5, 2023
- Account for view extents that cross the prime meridian
- Tweak the max num of geohashes to display in Geohash layer

Issues: #2119, #1720
@robyngit
Copy link
Member Author

robyngit commented May 5, 2023

📢 The test site has been updated for anyone who would like to try out the latest version: https://test.arcticdata.io/data

@robyngit robyngit modified the milestones: 2.24.0, 2.25.0 May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant