Support concurrent retrieval/decoding of images

[LcTwisk]

Hey,

First of all, thanks for this awesome project 🙏

While trying to retrieve a mix of static and animating images, I noticed that the decoding of the animated images (which take about a second to load) seems to 'block' the static images from loading (which usually load instantly). After digging around in the implementation I found that the image processing operation is dispatched on a queue that is initialized here:

Kingfisher/Sources/Cache/ImageCache.swift

Line 181 in 6faf189

ioQueue = DispatchQueue(label: ioQueueName)

.

A DispatchQueue is serial by default if no parameters (other than label) are being passed in, which explains my observation of images appearing one after the other in a certain order. Changing this to a concurrent queue solves the issue for me:

-DispatchQueue(label: ioQueueName)
+DispatchQueue(label: ioQueueName, attributes: .concurrent)

Can you think of any downside of using a concurrent instead of serial queue here? If not, I'd be happy to file a PR to change the queue strategy (could also be an opt-in feature of course).

Please let me know! 😁

[onevcat]

Oh. It is surprising that why image cache would causes this. Are encountering it when you try to load images from disk? So the bottleneck is the reading speed of the data?

If so, I guess we can make a concurrent queue for reading (but not for writing, which may cause issues).

could also be an opt-in feature of course

Yes, let's make it an opt-in first and see how it works. I suggest a concurrentReading: Bool in ImageCache and use a new concurrent queue for reading purpose from disk.

[LcTwisk]

Yes, it’s specifically a problem for images loading from disk. I don’t think the bottleneck is the reading speed though, it’s probably the custom image processor that we’re using for WebP images (triggered [here](

Kingfisher/Sources/Cache/ImageCache.swift

Line 583 in 6faf189

image = options.cacheSerializer.image(with: data, options: options)

, which runs on the same queue) ). I’m fine with concurrentReading, should we expose it just like one of the other Kingfisher options?

[onevcat]

Oh! That makes sense!

Then, instead of changing the current ioQueue, I prefer another queue such as cacheDeserializerQueue for that purpose. Also, concurrentReading is not a good name for that purpose. Maybe .concurrentCacheDeserializing is better.

should we expose it just like one of the other Kingfisher options

Yes. Let's just add a another Bool to the option in the same way! Thank you.

[LcTwisk]

Sounds good! That’s a better name indeed. Will open a PR in the coming days.

Thanks for the quick response 👍

[tioazhh]

Hey,

Thanks for the detailed analysis and your kind words! 🙏

You’re absolutely right about the issue with the queue blocking the static images. Switching to a concurrent queue seems like a reasonable solution to improve loading performance without any major downsides, as long as the image processing doesn’t rely on strict order or synchronization. I think it could work well, and it would be awesome if you could file a PR for this change!
Feel free to go ahead, and thanks again for your contribution! 😁
Read more