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

Avoid multiple parallel network requests for the same URL #1461

Open
Kazachenko1998 opened this issue Sep 16, 2022 · 7 comments
Open

Avoid multiple parallel network requests for the same URL #1461

Kazachenko1998 opened this issue Sep 16, 2022 · 7 comments
Labels
enhancement New feature or request

Comments

@Kazachenko1998
Copy link

I am developing an application where there can be many identical pictures on one screen (SVG from the server).
When I open the application for the first time, I see multiple parallel requests behind the same picture.

I think it's worth adding a pool of downloads, and if this URL is already being processed, do not make this request again, but wait for the first one to complete and use its result.

override fun onCreate(savedInstanceState: Bundle?) {
        CODE
        Coil.setImageLoader(
            ImageLoader
                .Builder(this)
                .components { add(SvgDecoder.Factory()) }
                .crossfade(true)
                .build()
        )
        CODE
}

AsyncImage(
    placeholder = rememberAsyncImagePainter(R.drawable.ic_plchldr),
    error = rememberAsyncImagePainter(R.drawable.ic_plchldr),
    model = URL,
    contentDescription = URL,
    contentScale = ContentScale.Crop,
    modifier = Modifier.fillMaxSize()
)

image

@Kazachenko1998 Kazachenko1998 added the enhancement New feature or request label Sep 16, 2022
@colinrtwhite
Copy link
Member

colinrtwhite commented Sep 18, 2022

I agree this would be nice to have. Ideally, HttpUriFetcher should reference count the number of responses waiting for its network request. We probably can't merge requests that only share the same URL - we'll need request headers to match as well (at least by default).

Additionally - we can only do this for requests that are going to be written to the disk cache. If they're not being written to the disk cache then we can only read the response body once. There's a few other edge cases that means this would have to be opt-in at least for a while.

@colinrtwhite colinrtwhite changed the title Do not load the same URLs at the same time Avoid multiple parallel network requests for the same URL Sep 18, 2022
@colinrtwhite
Copy link
Member

colinrtwhite commented Sep 28, 2022

Thinking about this more we should also provide a configurable "keep alive" duration for network requests. That way the user can configure how long to wait before cancelling a network request if there's no more consumer for the result.

This would be useful for RecyclerViews and other cases where a request can be restarted multiple times in quick succession.

@gargVader
Copy link

What is the status on this?

@Airsaid
Copy link

Airsaid commented Aug 5, 2024

How is the progress on this matter?

@gorkijatt
Copy link

gorkijatt commented Sep 22, 2024

I had the same issue with viewpager of images..

when first image loaded there are 3 requests of the same url.. this is due to the recomposition nature of composes

but it can be fixed easily by using remember with imagerequest

// Memorize the ImageRequest to prevent multiple requests on recompositions
val imageRequest = remember(portrait1080) {
    ImageRequest.Builder(LocalContext.current)
        .data(portrait1080)
        .allowHardware(false)
        .size(Size.ORIGINAL)  // Load the image at its original size
        .precision(Precision.EXACT)
        .listener(
            onStart = {
                println("loading image:$portrait1080")
                isLoading = true
            },
            onSuccess = { _, _ ->
                isLoading = false
                loadError = false
            },
            onError = { _, _ ->
                isLoading = false
                loadError = true
            }
        )
        .crossfade(true)
        .build()
}

// Load the image using the cached ImageRequest
AsyncImage(
    model = imageRequest,
    contentDescription = null,
    modifier = Modifier
        .fillMaxSize()
        .clickable { toggleStatusBarVisibility() },
    contentScale = ContentScale.Crop
)

@Airsaid
Copy link

Airsaid commented Sep 27, 2024

How is the progress on this matter?

I tried to fix the issue through an interceptor:

/**
 * Merge the same requests to avoid multiple parallel network requests for the same URL.
 *
 * You can set the [mergeFilter] to determine which requests need to be merged,
 * the default is to merge the network requests.
 *
 * You can set the [keyGenerator] to generate the key for the request,
 * the default is to use the request data as the key.
 *
 * Fix the [issue](https://github.com/coil-kt/coil/issues/1461).
 *
 * @param mergeFilter A function that determines which requests need to be merged.
 * @param keyGenerator A function that generates the key for the request.
 * @author airsaid
 */
class MergeRequestInterceptor(
  private val mergeFilter: (ImageRequest) -> Boolean = {
    it.isNetUrlRequest()
  },
  private val keyGenerator: (ImageRequest) -> String = {
    it.generateKey()
  }
) : Interceptor {

  private val pendingRequests = ConcurrentHashMap<String, CompletableDeferred<ImageResult>>()

  override suspend fun intercept(chain: Interceptor.Chain): ImageResult {
    val req = chain.request
    if (!mergeFilter(req)) {
      return chain.proceed(req)
    }

    // If there is a pending request, we should wait for it
    val requestKey = keyGenerator(req)
    val existingRequest = pendingRequests[requestKey]
    return if (existingRequest != null) {
      try {
        existingRequest.await()
      } catch (ignore: Exception) {
        // Ignore the exception to keep the request chain running
      }
      // We can't return the result of the existing request directly,
      // because it will not be shown in the UI for the same result.
      // Therefore, we should transfer the request and return a different cached result
      chain.proceed(chain.request)
    } else {
      val deferred = CompletableDeferred<ImageResult>()
      pendingRequests[requestKey] = deferred
      try {
        // Proceed with the actual request
        val result = chain.proceed(chain.request)
        deferred.complete(result)
        result
      } catch (e: Exception) {
        deferred.completeExceptionally(e)
        throw e
      } finally {
        // Remove the request from the pending map
        pendingRequests.remove(requestKey)
      }
    }
  }
}

private fun ImageRequest.isNetUrlRequest(): Boolean {
  val data = data
  if (data is Uri) {
    return data.isNetworkUri
  }
  if (data is String) {
    val uri = Uri.parse(data)
    return uri.isNetworkUri
  }
  return false
}

private fun ImageRequest.generateKey(): String {
  return data.toString()
}

@Firsto
Copy link

Firsto commented Nov 20, 2024

How is the progress on this matter?

I tried to fix the issue through an interceptor:

I tried to use it. But...
Not working anyway.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants