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

Custom Fetcher Memory Loading is really slow #2770

Closed
vanniktech opened this issue Jan 5, 2025 · 8 comments · Fixed by #2789
Closed

Custom Fetcher Memory Loading is really slow #2770

vanniktech opened this issue Jan 5, 2025 · 8 comments · Fixed by #2789

Comments

@vanniktech
Copy link

I have the same behavior as here: #629

Basically when you scroll down and up, the image is recycled and then needs to be loaded again. However, it takes a considerable amount of time (at least 2-3 seconds on my Pixel 6) which is too much for a memory cache, right?

I can see in logcat that the Memory Cache is used: 🧠 Successful (MEMORY)

This is how I return the image that I want to display from my custom fetcher:

  private suspend fun inMemory(statement: HttpStatement) = SourceFetchResult(
    source = ImageSource(
      source = statement.execute().bodyAsChannel().toBufferedSource(),
      fileSystem = options.fileSystem,
    ),
    mimeType = profileAvatarRequest.mimeType,
    dataSource = DataSource.MEMORY,
  )

Am I doing something wrong?

I'd expect the image to be rendered instantly when I scroll up again, instead I see my placeholder being shown for 2-3 seconds.

@colinrtwhite
Copy link
Member

This sounds like it could be a case where the image isn't being returned from the memory cache. What do the extra debug logs say?

I can see in logcat that the Memory Cache is used: 🧠 Successful (MEMORY)

This actually means the image was returned from a memory source i.e. your fetcher as it returns DataSource.MEMORY. There's a separate DataSource.MEMORY_CACHE that indicates the memory cache.

@vanniktech
Copy link
Author

I've added .logger(DebugLogger()) and I'm not seeing much more than this. I'm only using io.coil-kt.coil3:coil-compose though.

@colinrtwhite
Copy link
Member

It sounds like your request is missing the memory cache and is loading via your custom fetcher which takes 2-3 seconds, though it's not possible to say without a way to repro locally. It's possible it could be this, but there would be a log indicating why the memory cache result was ignored in that case.

@vanniktech
Copy link
Author

vanniktech commented Jan 7, 2025

Yes the override suspend fun fetch(): FetchResult? { is called multiple times but there is no log statement. No worries, I've managed to reproduce this here: vanniktech/playground#308

If you scroll down, and up again you can see that requests are loaded again and the fetcher is called.

@vanniktech
Copy link
Author

@colinrtwhite have you had time to check out the reproducer?

@colinrtwhite
Copy link
Member

@vanniktech I took a look at the repro sample and the issue is you need to also register a Keyer<Int> to ensure a memory cache key is created based on the Int. This is mentioned in the docs here, but I've added more docs here to make it more prominent.

@vanniktech
Copy link
Author

Ah yes, now i'm getting: 🧠 Successful (MEMORY_CACHE)

Isn't possible to emit a warning if you have a custom fetcher, but you haven't provided a keyer for the same type? Coil has all of the information, so it should be fairly trivial, right?

@colinrtwhite
Copy link
Member

@vanniktech Agreed. I can't remember why we don't default to data.toString() in the case of a missing Keyer, but I feel like there's a good reason. Going to think through changing the default on this, but it's tough as changing it could break someone's use case.

In the meantime I've added a more prominent warning here: #2816

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

Successfully merging a pull request may close this issue.

2 participants