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

How to determine failed downloads from preheating? #167

Closed
SleepiestAdam opened this issue Jun 11, 2018 · 14 comments
Closed

How to determine failed downloads from preheating? #167

SleepiestAdam opened this issue Jun 11, 2018 · 14 comments
Labels

Comments

@SleepiestAdam
Copy link

Hi,
So we're currently using Kingfisher, however we're noticing some performance issues on large collection views... So Nuke library seems like the ideal solution!

The only query I have is in regards to the preheating... We're currently trying to download all the images our app needs on-launch so that by the time the users reaches the screen where they're actually needed; they're already available.

We're keen for the app to work in areas where the user has weak signal; often meaning not all image requests are successful. Kingfisher's ImagePrefetcher caters for this by returning which images succeeded and failed downloading; we then log which ones failed and try again at an appropriate time.

I'm struggling to find anything similar in the Nuke documentation; is there a way to determine which images were and weren't successfully preheated? Also is there a means of specifying a count for the number of times to potentially re-try downloading a given image? Or a means of getting a callback when each image in the preheat list has failed / been downloaded successfully?

Many thanks!

@kean
Copy link
Owner

kean commented Jun 11, 2018

Hi,

however we're noticing some performance issues on large collection views

Nuke is faster on the main thread than Kingfisher but I would definitely recommend profiling your app first to make sure what the cause of the performance issues actually is.

There is an ImagePreheater type in Nuke which is designed to be used in table and collection views to prefetch images which are expected to appear on the display soon. It's designed to be used on fast internet to make sure the user never sees any loading indicators or empty image views. That's how I personally use it and I imagine this use case is very different from the one that you have.

I haven't extended ImagePreheater functionality much from the day it was introduced. There is definitely room for adding more features to it - PRs are welcome.

But in your specific use case I would recommend implementing this by hand. There isn't much preheater does for you anyway. It can be as simple as this:

func startPrefetchingImages(for urls: [URL]) {
    func prefetchImage(for request: ImageRequest) {
        ImagePipeline.shared.loadImage(with: request) { (response, error) in
            if response == nil {
                // Retry after 10 seconds
                DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(10)) {
                    prefetchImage(for: request)
                }
            }
        }
    }

    for url in urls {
        var request = ImageRequest(url: url)
        request.priority = .low // Lower than regular requests

        prefetchImage(for: request)
    }
}

The retry logic can be as complex as you'd like it to be. I personally use RxNuke with smart retries.

@SleepiestAdam
Copy link
Author

Hey - I have profiled it, and it's deff kingfisher that's the culprit (I actually started delving in to clean up some kingfisher code and got it running a fair bit smoother for my specific use-case, however having looked at Nuke and the benchmarks; it seemed like the better option long-term was simply to just switch over, particularly due to the reduced download time of the assets using the WebP extension).

Thanks for pointing me in the right direction :)!

@SleepiestAdam
Copy link
Author

Hey Kean; think I'm almost there! Just one issue I'm running into still...

On older devices it's taking quite a while to display a cached image (often 2-5 seconds). I gather this might be related to the decodingQueue?

I'm going to be implementing UICollectionViewDataSourcePrefetching throughout; but are there any other ways I can improve the speed with which it gets / decodes the cached images?

Weirdly on initial launch it's actually a better user experience, as it's pre-loads them in the background. On subsequent launches I've got it checking if they're cached ahead of trying to load them / if not it just waits until the user is entering the view containing the image before it starts loading from the cache (I assumed it'd be a few ms, but looks like I was wrong as the result is annoying delay on older devices.)

Alternatively; is there a way to prioritise things in the decoding que so I can just bump item the user is trying to view at the specific moment to the front of the que?

Many thanks!

@kean
Copy link
Owner

kean commented Jun 12, 2018

Hi,

I gather this might be related to the decodingQueue?

It might be it. But it's more likely that the problem is background decompression (see ImageDecompressor), not decoding (Data -> Image). I suggest taking a look at ImageTaskMetrics to debug performance issues.

If you load lots of images ahead of time I would recommend disabling some of the pipeline steps. Here's what you can do using existing APIs:

var request = ImageRequest(url: url)
request.processor = nil // Don't decompress the image
request.memoryCacheOptions.isWriteAllowed = false // Disable mem cache (optional)

The image is still going to be decoded (Data -> Image, quick) but it's not going to be decompressed (slow). The "deduplication" is smart enough so that it won't download the same image twice if you then start a request with the same URL but with memory cache enabled & processor enabled.

Alternatively; is there a way to prioritise things in the decoding que so I can just bump item the user is trying to view at the specific moment to the front of the que?

I have a couple of ideas in backlog regarding improving prefetching.

First, there needs to be an API to skip decoding. I was thinking adding something semantic like func prefetchImage(.inDiskCache | .inMemoryCache) to ImagePipeline itself. This needs to be a separate API because if you skip decoding, no image is produced - can't complete with "success" ImageResponse.

Second, the decoding & processing queues should respect the request priorities - at the moment only data loading queue does.

@SleepiestAdam
Copy link
Author

Mmm ok, so I've used the metrics, and I'm getting the following in relation to one of the slower-loading images:

Task Information {
    Task ID - 203
    Duration - 11:18:47.823 – 11:19:00.230 (12.406s)
    11:19:00.198 – 11:19:00.229 (0.031s) - Process
    Was Cancelled - false
    Is Memory Cache Hit - false
    Was Subscribed To Existing Image Loading Session - false
}
Image Loading Session {
    Session Information {
        Session ID - 88
        Total Duration - nil
        Was Cancelled - false
    }
    Timeline {
        11:18:47.824 – nil (unknown)         - Total
        ------------------------------------
        nil – nil (nil)                      - Check Disk Cache
        11:18:49.129 – 11:18:49.138 (0.008s) - Load Data
        11:18:59.525 – 11:19:00.197 (0.671s) - Decode
    }
    Resumable Data {
        Was Resumed - nil
        Resumable Data Count - nil
        Server Confirmed Resume - nil
    }
}

It looks like loading the data is fairly quick, and de-coding it is passable, however the 'Duration' is still 12.406s by the time the image (on the page I'm looking at) gets loaded... I gather this is due to the images in the que ahead of it.

Is there a simple way to make the que progress quicker / run more stuff concurrently?
Or would simply using a separate ImagePipeline for each "section" of the app likely yield some benefit . in this instance?

@kean
Copy link
Owner

kean commented Jun 12, 2018

So all the time is spent waiting in a decoding queue. That's weird, it's not normal for decoding to take so long (0.671 seconds). Are you loading WebP images? This might explain why it takes so long. If so, I could create a patch today that would make decoding & processing queues respect requests' priorities - this would fix your issue.

This would require switching from DispatchQueue to OperationQueue for decoding queue. I can make if configurable via ImagePipeline.Configuration so that you could play with maximum concurrent task count.

@SleepiestAdam
Copy link
Author

I am indeed using WebP images / think that's the root of the issue... If you could add the ability for it to take priority into account with decoding then that would be fantastic / a huge help... presumably then I could just re-call the loadImage with a request priority of .veryHigh in order for it to jump to the top of the que?

Being able to play with the maximum concurrent would also be awesome, that way I can potentially have a play and figure out the optimum concurrent tasks's for the older devices.

@kean
Copy link
Owner

kean commented Jun 12, 2018

presumably then I could just re-call the loadImage with a request priority of .veryHigh in order for it to jump to the top of the que?

That's correct.

@SleepiestAdam
Copy link
Author

Ok yep; I'm thinking:

  1. I make the non-transparent images into JPG's on the server assuming they'll be decoded much quicker from the cache.

  2. There's still a lot of images that require alpha (hence using WebP for them rather than png as for alpha they're like 1/10 of the size)... So for those if I use your patch to just prioritise them if they're on screen (alongside implementing UICollectionViewDataSourcePrefetching)... Hopefully that should improve UX a fair bit.

@kean
Copy link
Owner

kean commented Jun 12, 2018

  1. I make the non-transparent images into JPG's on the server assuming they'll be decoded much quicker from the cache.

The reason why JPEG "decoding" (Data -> UIImage) is fast is that UIImage doesn't actually decompress the data until it needs to. The reason why ImageDecompressor exists is to force decompression in a background. If you don't, the decompression is going to happen on the main thread when you display the image which would result in dropped frames.

So for JPEG most of the work happens in ImageDecompressor, not ImageDecoder. If you switch to JPEG you might end up just moving the work from "decoding" stage to "processing" stage.

I don't have much recent experience with WebP, but my guess is that it eagerly decompresses the data. This is why decoding takes so long.

  • decoding - creating UIImage from Data
  • decompression - forcing UIImage to render its contents in ImageDecompressor, this happens during the "processing" stage

I would wait for a patch and see if it improves the situation.

@kean
Copy link
Owner

kean commented Jun 12, 2018

Pushed a new version with these changes. It works well but there is some technical debt which I'm going to address in 7.2.1. Hope that helps, please close the issue if it does and create a new one if you find some other issues.

@SleepiestAdam
Copy link
Author

Ok, so it's working perfectly now!

The patch above deff helped... That said, WebP decoding was still really slow on older devices (iPad Air 2 / iPad 4th gen etc), to the point it would often still take a few seconds to load the 10-15 WebP images I had displayed on the given page.

For anyone else that runs into the same issue with WebP:

We're basically just downloading loads of app content on launch which for the most part remains the same once downloaded. The solution for us was to add a layer of our own cacheing whereby once the WebP image is downloaded and decoded for the first time we use UIImagePNGRepresentation to cache it on disk as a png file.

We then used Nuke to simply load the png from local on subsequent re-opens of the app, which decodes orders of magnitude quicker than the WebP image, and even on the old devices makes it work perfectly.

Bit hacky, but it does the job well.

@kean
Copy link
Owner

kean commented Jun 18, 2018

hey @AssyriaGameStudio, I've addressed tech debt introduced in version 7.1 and 7.2 and also made some improvements in the way operations are schedules, reused, canceled, and how priorities are updated. Now it should be pretty much perfect. If you spot any other performance issues, please let me know.

@zhihuitang
Copy link

zhihuitang commented Aug 14, 2018

Hi @kean

Theorically, if ImagePipeline.shared.loadImage failed, it will try forever? Is my understanding correct?

func startPrefetchingImages(for urls: [URL]) {
    func prefetchImage(for request: ImageRequest) {
        ImagePipeline.shared.loadImage(with: request) { (response, error) in
            if response == nil {
                // Retry after 10 seconds
                DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(10)) {
                    prefetchImage(for: request)
                }
            }
        }
    }

    for url in urls {
        var request = ImageRequest(url: url)
        request.priority = .low // Lower than regular requests

        prefetchImage(for: request)
    }
}

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

No branches or pull requests

3 participants