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.
This commit is contained in:
Jason-Whitmore 2025-02-18 22:42:24 -08:00 committed by Nicolas Raoul
parent bb0f2bcbd0
commit 0272a3ee73
3 changed files with 73 additions and 13 deletions

View file

@ -232,13 +232,23 @@ public class Place implements Parcelable {
*/ */
@Nullable @Nullable
public String getWikiDataEntityId() { public String getWikiDataEntityId() {
if (this.entityID != null && !this.entityID.equals("")) {
return this.entityID;
}
if (!hasWikidataLink()) { if (!hasWikidataLink()) {
Timber.d("Wikidata entity ID is null for place with sitelink %s", siteLinks.toString()); Timber.d("Wikidata entity ID is null for place with sitelink %s", siteLinks.toString());
return null; return null;
} }
//Determine entityID from link
String wikiDataLink = siteLinks.getWikidataLink().toString(); 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;
} }
/** /**

View file

@ -11,6 +11,7 @@ import android.content.IntentFilter
import android.content.res.Configuration import android.content.res.Configuration
import android.graphics.Color import android.graphics.Color
import android.graphics.Paint import android.graphics.Paint
import android.graphics.drawable.Drawable
import android.location.Location import android.location.Location
import android.location.LocationManager import android.location.LocationManager
import android.net.Uri import android.net.Uri
@ -219,6 +220,7 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(), NearbyParentFragmen
@Volatile @Volatile
private var stopQuery = false private var stopQuery = false
private var drawableCache: MutableMap<Pair<Context, Int>, Drawable>? = null
// Explore map data (for if we came from Explore) // Explore map data (for if we came from Explore)
private var prevZoom = 0.0 private var prevZoom = 0.0
@ -721,6 +723,7 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(), NearbyParentFragmen
startMapWithoutPermission() startMapWithoutPermission()
} }
} }
drawableCache = HashMap()
} }
/** /**
@ -1501,7 +1504,7 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(), NearbyParentFragmen
marker.showInfoWindow() marker.showInfoWindow()
presenter!!.handlePinClicked(updatedPlace) presenter!!.handlePinClicked(updatedPlace)
savePlaceToDatabase(place) savePlaceToDatabase(place)
val icon = ContextCompat.getDrawable( val icon = getDrawable(
requireContext(), requireContext(),
getIconFor(updatedPlace, isBookMarked) 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 { 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 point = GeoPoint(place.location.latitude, place.location.longitude)
val marker = Marker(binding!!.map) val marker = Marker(binding!!.map)
marker.position = point marker.position = point
@ -2430,7 +2460,7 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(), NearbyParentFragmen
Marker.ANCHOR_BOTTOM Marker.ANCHOR_BOTTOM
) )
startMarker.icon = startMarker.icon =
ContextCompat.getDrawable( getDrawable(
this.requireContext(), this.requireContext(),
fr.free.nrw.commons.R.drawable.current_location_marker fr.free.nrw.commons.R.drawable.current_location_marker
) )
@ -2488,7 +2518,7 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(), NearbyParentFragmen
Marker(binding?.map).apply { Marker(binding?.map).apply {
position = it position = it
setAnchor(Marker.ANCHOR_CENTER, Marker.ANCHOR_BOTTOM) 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" title = "Your Location"
textLabelFontSize = 24 textLabelFontSize = 24
overlays.add(this) overlays.add(this)

View file

@ -25,9 +25,12 @@ import kotlinx.coroutines.Job
import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.delay import kotlinx.coroutines.delay
import kotlinx.coroutines.ensureActive import kotlinx.coroutines.ensureActive
import kotlinx.coroutines.job
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext import kotlinx.coroutines.withContext
import okhttp3.internal.wait
import timber.log.Timber import timber.log.Timber
import java.io.IOException
import java.lang.reflect.InvocationHandler import java.lang.reflect.InvocationHandler
import java.lang.reflect.Method import java.lang.reflect.Method
import java.lang.reflect.Proxy import java.lang.reflect.Proxy
@ -75,8 +78,8 @@ class NearbyParentFragmentPresenter
* - **connnectionCount**: number of parallel requests * - **connnectionCount**: number of parallel requests
*/ */
private object LoadPlacesAsyncOptions { private object LoadPlacesAsyncOptions {
const val BATCH_SIZE = 3 const val BATCH_SIZE = 10
const val CONNECTION_COUNT = 3 const val CONNECTION_COUNT = 20
} }
private var schedulePlacesUpdateJob: Job? = null private var schedulePlacesUpdateJob: Job? = null
@ -91,7 +94,7 @@ class NearbyParentFragmentPresenter
private object SchedulePlacesUpdateOptions { private object SchedulePlacesUpdateOptions {
var skippedCount = 0 var skippedCount = 0
const val SKIP_LIMIT = 3 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 // used to tell the asynchronous place detail loading job that the places' bookmarked status
@ -379,13 +382,32 @@ class NearbyParentFragmentPresenter
) )
} catch (e: Exception) { } catch (e: Exception) {
Timber.tag("NearbyPinDetails").e(e) 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<Pair<Int, MarkerPlaceGroup>>()
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 var collectCount = 0
for (resultList in collectResults) { while (collectCount < indicesToUpdate.size) {
val resultList = collectResults.receive()
for ((index, fetchedPlaceGroup) in resultList) { for ((index, fetchedPlaceGroup) in resultList) {
val existingPlace = updatedGroups[index].place val existingPlace = updatedGroups[index].place
val finalPlaceGroup = MarkerPlaceGroup( val finalPlaceGroup = MarkerPlaceGroup(
@ -442,9 +464,7 @@ class NearbyParentFragmentPresenter
} }
} }
schedulePlacesUpdate(updatedGroups) schedulePlacesUpdate(updatedGroups)
if (++collectCount == totalBatches) { collectCount += resultList.size
break
}
} }
collectResults.close() collectResults.close()
} }