Fixes Issue 5933: Nearby: Display of all nearby pins makes the app sluggish, leads to crashes #6181
Conversation
|
Even after giving the app enough time to load the correct colored map markers, some still remain gray. I looked at the logs and discovered that there were some HTTP responses with error code 500. I believe this (server side?) bug is also stopping the rest of the places in the batch from being sent back to the client, resulting in every Place in the batch appearing gray. One example of a place that has this issue is right above "54 Broadway" in London, UK. Using the debugger, that pin is linked here. It seems like the entry is for the entire UK. There seem to be these kinds of places in most dense cities and should be somewhat easy to find. To avoid the server not returning a response for a whole batch, smaller batch sizes can be used to minimize the problem. However, smaller batch sizes require more requests, which may cause the server to return with the error code 429. Overall, I'm not entirely sure what combination of batch size and connection count is best. |
|
I believe you encountered #6064 Such items are very rare, and all have an image already. When a batch fails, I would suggest retrying that batch for each pin individually. The pin that triggers error 500 can be left grey, or even removed since it certainly already has an image. This can be left for a separate pull request, unless you want to do it here. 🙂 |
|
I am trying your branch, it is wonderfully fast! One issue: When there are many pins, I can often see some staying grey, even after waiting for 1 minute. This does not happen on the main branch (on main pins load slowly but they all load). Tapping each grey pin makes it turn green or red, so it is different from the error 500 issue. screen-20250211-115040b.mp4In the screencast above (this pull request's branch), I wait 1 minute then tap the grey pins, they all quickly turn red or green. |
|
I'll take a look at it and try to find a fix. Thanks! |
|
Sorry by coincidence |
Before this commit, this method would perform the String replace method on the Wikidata link every time getWikiDataEntityID() was called. Also, getWikiDataLink() was called. This caused poor performance since both method calls are slow. This commit changes the method to only run the slow methods if the entityID field is empty or null. Once the field is populated, the method simply returns the field. This change allows getWikiDataEntityID() to run much faster.
…eters Before this commit, the parameters that configure the async and place update code contributed to the slow loading of the red and green map markers. This commit changes the parameters such that the red and green map markers load much faster. These parameters may need further tuning. This commit's changes are simply an educated guess at a good parameter set.
Before this commit, the code which cached Drawables was written in Java. This commit rewrites that code into the new Kotlin file, which replaces the Java file.
… HTTP requests after failure Before this commit, when an HTTP request failed, the entire batch of Places would not get updated, even if it was only one Place in the batch that caused the failure. This commit changes the code such that upon HTTP request failure, new HTTP requests are sent with one Place per request. This allows more Places to be fetched from the server.
Jason-Whitmore
force-pushed
the
issue_5933
branch
from
7217f9c to
d735e1c
Compare
|
@nicolas-raoul I've added commits which transfer the new getDrawable code to the Kotlin file and sends new HTTP requests for individual places after the batched HTTP request fails. During testing, this seems to work for handling 500 errors that occur in the batched HTTP request. However, 429 errors still prevent some gray pins from turning red or green. This problem could probably be fixed, but I'm not sure where the solution could be placed. My guess is that code in Let me know what you think. Thanks! |
Description (required)
Fixes #5933
What changes did you make and why?
Two nontrivial changes were made to Place.java and NearbyPlaceFragment.java. These changes cache/save results of method calls in order to speed up the retrieval of Drawables (map markers) and WikiData Entity IDs (for Places).
Trivial changes were made to NearbyPlaceFragmentPresenter.kt. The parameters that control the frequency and batch size of HTTP requests were increased to retrieve information faster. The SKIP_DELAY_MS parameter was changed to increase the frequency of updates to the map. I'm not sure what combination of parameters are the best, but the values I've set seem to work well.
All of these changes increase the rate at which the colored map markers load.
Tests performed (required)
Tested ProdRelease on Android Studio Emulator with API level 34.
Screenshots (for UI changes only)
issue_5933_compressed.webm