From 98b25acab9c6c515975cbed0b3dc950b05098dc5 Mon Sep 17 00:00:00 2001 From: Jason-Whitmore Date: Tue, 18 Feb 2025 22:42:24 -0800 Subject: [PATCH] Fixes Issue 5933: Nearby: Display of all nearby pins makes the app sluggish, leads to crashes (#6181) * Place.java: change getWikiDataEntityID() method to increase speed 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. * NearbyParentFragmentPresenter.kt: change async and place update parameters 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. * NearbyParentFragment.kt: rewrite Java Drawable caching code to Kotlin 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. * NearbyParentFragmentPresenter.kt: change loadPlacesDataAsync to retry 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. --- .../fr/free/nrw/commons/nearby/Place.java | 12 +++++- .../nearby/fragments/NearbyParentFragment.kt | 38 +++++++++++++++++-- .../NearbyParentFragmentPresenter.kt | 36 ++++++++++++++---- 3 files changed, 73 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/Place.java b/app/src/main/java/fr/free/nrw/commons/nearby/Place.java index 21dd14131..3b3b798eb 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/Place.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/Place.java @@ -232,13 +232,23 @@ public class Place implements Parcelable { */ @Nullable public String getWikiDataEntityId() { + if (this.entityID != null && !this.entityID.equals("")) { + return this.entityID; + } + if (!hasWikidataLink()) { Timber.d("Wikidata entity ID is null for place with sitelink %s", siteLinks.toString()); return null; } + //Determine entityID from link String wikiDataLink = siteLinks.getWikidataLink().toString(); - return wikiDataLink.replace("http://www.wikidata.org/entity/", ""); + + if (wikiDataLink.contains("http://www.wikidata.org/entity/")) { + this.entityID = wikiDataLink.substring("http://www.wikidata.org/entity/".length()); + return this.entityID; + } + return null; } /** diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.kt b/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.kt index 90167f8e3..d2e73441d 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.kt +++ b/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.kt @@ -11,6 +11,7 @@ import android.content.IntentFilter import android.content.res.Configuration import android.graphics.Color import android.graphics.Paint +import android.graphics.drawable.Drawable import android.location.Location import android.location.LocationManager import android.net.Uri @@ -219,6 +220,7 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(), NearbyParentFragmen @Volatile private var stopQuery = false + private var drawableCache: MutableMap, Drawable>? = null // Explore map data (for if we came from Explore) private var prevZoom = 0.0 @@ -721,6 +723,7 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(), NearbyParentFragmen startMapWithoutPermission() } } + drawableCache = HashMap() } /** @@ -1501,7 +1504,7 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(), NearbyParentFragmen marker.showInfoWindow() presenter!!.handlePinClicked(updatedPlace) savePlaceToDatabase(place) - val icon = ContextCompat.getDrawable( + val icon = getDrawable( requireContext(), getIconFor(updatedPlace, isBookMarked) ) @@ -2027,8 +2030,35 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(), NearbyParentFragmen ) } + /** + * Gets the specified Drawable object. This is a wrapper method for ContextCompat.getDrawable(). + * This method caches results from previous calls for faster retrieval. + * + * @param context The context to use to get the Drawable + * @param id The integer that describes the Drawable resource + * @return The Drawable object + */ + private fun getDrawable(context: Context?, id: Int?): Drawable? { + if (drawableCache == null || context == null || id == null) { + return null + } + + val key = Pair(context, id) + if (!drawableCache!!.containsKey(key)) { + val drawable = ContextCompat.getDrawable(context, id) + + if (drawable != null) { + drawableCache!![key] = drawable + } else { + return null + } + } + + return drawableCache!![key] + } + fun convertToMarker(place: Place, isBookMarked: Boolean): Marker { - val icon = ContextCompat.getDrawable(requireContext(), getIconFor(place, isBookMarked)) + val icon = getDrawable(requireContext(), getIconFor(place, isBookMarked)) val point = GeoPoint(place.location.latitude, place.location.longitude) val marker = Marker(binding!!.map) marker.position = point @@ -2430,7 +2460,7 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(), NearbyParentFragmen Marker.ANCHOR_BOTTOM ) startMarker.icon = - ContextCompat.getDrawable( + getDrawable( this.requireContext(), fr.free.nrw.commons.R.drawable.current_location_marker ) @@ -2488,7 +2518,7 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(), NearbyParentFragmen Marker(binding?.map).apply { position = it setAnchor(Marker.ANCHOR_CENTER, Marker.ANCHOR_BOTTOM) - icon = ContextCompat.getDrawable(context, R.drawable.current_location_marker) + icon = getDrawable(context, R.drawable.current_location_marker) title = "Your Location" textLabelFontSize = 24 overlays.add(this) diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.kt b/app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.kt index ce268f7a3..ed84751b0 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.kt +++ b/app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.kt @@ -25,9 +25,12 @@ import kotlinx.coroutines.Job import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.delay import kotlinx.coroutines.ensureActive +import kotlinx.coroutines.job import kotlinx.coroutines.launch import kotlinx.coroutines.withContext +import okhttp3.internal.wait import timber.log.Timber +import java.io.IOException import java.lang.reflect.InvocationHandler import java.lang.reflect.Method import java.lang.reflect.Proxy @@ -75,8 +78,8 @@ class NearbyParentFragmentPresenter * - **connnectionCount**: number of parallel requests */ private object LoadPlacesAsyncOptions { - const val BATCH_SIZE = 3 - const val CONNECTION_COUNT = 3 + const val BATCH_SIZE = 10 + const val CONNECTION_COUNT = 20 } private var schedulePlacesUpdateJob: Job? = null @@ -91,7 +94,7 @@ class NearbyParentFragmentPresenter private object SchedulePlacesUpdateOptions { var skippedCount = 0 const val SKIP_LIMIT = 3 - const val SKIP_DELAY_MS = 500L + const val SKIP_DELAY_MS = 100L } // used to tell the asynchronous place detail loading job that the places' bookmarked status @@ -379,13 +382,32 @@ class NearbyParentFragmentPresenter ) } catch (e: Exception) { Timber.tag("NearbyPinDetails").e(e) - collectResults.send(indices.map { Pair(it, updatedGroups[it]) }) + //HTTP request failed. Try individual places + for (i in indices) { + launch { + val onePlaceBatch = mutableListOf>() + try { + val fetchedPlace = nearbyController.getPlaces( + mutableListOf(updatedGroups[i].place) + ) + + onePlaceBatch.add(Pair(i, MarkerPlaceGroup( + bookmarkLocationDao.findBookmarkLocation( + fetchedPlace[0]),fetchedPlace[0]))) + } catch (e: Exception) { + Timber.tag("NearbyPinDetails").e(e) + onePlaceBatch.add(Pair(i, updatedGroups[i])) + } + collectResults.send(onePlaceBatch) + } + } } } } } var collectCount = 0 - for (resultList in collectResults) { + while (collectCount < indicesToUpdate.size) { + val resultList = collectResults.receive() for ((index, fetchedPlaceGroup) in resultList) { val existingPlace = updatedGroups[index].place val finalPlaceGroup = MarkerPlaceGroup( @@ -442,9 +464,7 @@ class NearbyParentFragmentPresenter } } schedulePlacesUpdate(updatedGroups) - if (++collectCount == totalBatches) { - break - } + collectCount += resultList.size } collectResults.close() }