From 50991bb6129b919389c94138951b5af966c4f2c0 Mon Sep 17 00:00:00 2001 From: onevcat Date: Sat, 9 Feb 2019 22:33:19 +0900 Subject: [PATCH 1/2] Use the same queue for all prefetchers --- Sources/Networking/ImagePrefetcher.swift | 13 +++++-------- Tests/KingfisherTests/ImagePrefetcherTests.swift | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/Sources/Networking/ImagePrefetcher.swift b/Sources/Networking/ImagePrefetcher.swift index 872847e93..bd5f34626 100644 --- a/Sources/Networking/ImagePrefetcher.swift +++ b/Sources/Networking/ImagePrefetcher.swift @@ -31,6 +31,10 @@ import AppKit import UIKit #endif +// The dispatch queue to use for handling resource process, so downloading does not occur on the main thread +// This prevents stuttering when preloading images in a collection view or table view. +private let prefetchQueue = DispatchQueue(label: "com.onevcat.Kingfisher.PrefetchQueue") + /// Progress update block of prefetcher. /// /// - `skippedResources`: An array of resources that are already cached before the prefetching starting. @@ -57,10 +61,7 @@ public class ImagePrefetcher { /// The maximum concurrent downloads to use when prefetching images. Default is 5. public var maxConcurrentDownloads = 5 - - // The dispatch queue to use for handling resource process, so downloading does not occur on the main thread - // This prevents stuttering when preloading images in a collection view or table view. - private var prefetchQueue: DispatchQueue + private let prefetchResources: [Resource] private let optionsInfo: KingfisherParsedOptionsInfo @@ -138,10 +139,6 @@ public class ImagePrefetcher { prefetchResources = resources pendingResources = ArraySlice(resources) - // Set up the dispatch queue that all our work should occur on. - let prefetchQueueName = "com.onevcat.Kingfisher.PrefetchQueue" - prefetchQueue = DispatchQueue(label: prefetchQueueName) - // We want all callbacks from our prefetch queue, so we should ignore the callback queue in options. // Add our own callback dispatch queue to make sure all internal callbacks are // coming back in our expected queue. diff --git a/Tests/KingfisherTests/ImagePrefetcherTests.swift b/Tests/KingfisherTests/ImagePrefetcherTests.swift index c99f93a22..1b25e2b7b 100644 --- a/Tests/KingfisherTests/ImagePrefetcherTests.swift +++ b/Tests/KingfisherTests/ImagePrefetcherTests.swift @@ -298,4 +298,20 @@ class ImagePrefetcherTests: XCTestCase { waitForExpectations(timeout: 3, handler: nil) } + + func testPrefetchMultiTimes() { + let exp = expectation(description: #function) + let group = DispatchGroup() + + testURLs.forEach { stub($0, data: testImageData) } + for _ in 0..<10000 { + group.enter() + let prefetcher = ImagePrefetcher(resources: testURLs) { _, _, _ in + group.leave() + } + prefetcher.start() + } + group.notify(queue: .main) { exp.fulfill() } + waitForExpectations(timeout: 3, handler: nil) + } } From e23a2eaf7ccea3f19004cdb728296699c58f4aaa Mon Sep 17 00:00:00 2001 From: onevcat Date: Sat, 9 Feb 2019 22:40:14 +0900 Subject: [PATCH 2/2] Remove prefetch queue since it is legacy --- Sources/Networking/ImagePrefetcher.swift | 73 ++++++++++-------------- 1 file changed, 30 insertions(+), 43 deletions(-) diff --git a/Sources/Networking/ImagePrefetcher.swift b/Sources/Networking/ImagePrefetcher.swift index bd5f34626..885ecb883 100644 --- a/Sources/Networking/ImagePrefetcher.swift +++ b/Sources/Networking/ImagePrefetcher.swift @@ -31,10 +31,6 @@ import AppKit import UIKit #endif -// The dispatch queue to use for handling resource process, so downloading does not occur on the main thread -// This prevents stuttering when preloading images in a collection view or table view. -private let prefetchQueue = DispatchQueue(label: "com.onevcat.Kingfisher.PrefetchQueue") - /// Progress update block of prefetcher. /// /// - `skippedResources`: An array of resources that are already cached before the prefetching starting. @@ -156,45 +152,38 @@ public class ImagePrefetcher { /// Starts to download the resources and cache them. This can be useful for background downloading /// of assets that are required for later use in an app. This code will not try and update any UI /// with the results of the process. - public func start() - { - // Since we want to handle the resources cancellation in the prefetch queue only. - prefetchQueue.async { - - guard !self.stopped else { - assertionFailure("You can not restart the same prefetcher. Try to create a new prefetcher.") - self.handleComplete() - return - } - - guard self.maxConcurrentDownloads > 0 else { - assertionFailure("There should be concurrent downloads value should be at least 1.") - self.handleComplete() - return - } + public func start() { + guard !stopped else { + assertionFailure("You can not restart the same prefetcher. Try to create a new prefetcher.") + handleComplete() + return + } - // Empty case. - guard self.prefetchResources.count > 0 else { - self.handleComplete() - return - } - - let initialConcurrentDownloads = min(self.prefetchResources.count, self.maxConcurrentDownloads) - for _ in 0 ..< initialConcurrentDownloads { - if let resource = self.pendingResources.popFirst() { - self.startPrefetching(resource) - } + guard maxConcurrentDownloads > 0 else { + assertionFailure("There should be concurrent downloads value should be at least 1.") + handleComplete() + return + } + + // Empty case. + guard prefetchResources.count > 0 else { + handleComplete() + return + } + + let initialConcurrentDownloads = min(prefetchResources.count, maxConcurrentDownloads) + for _ in 0 ..< initialConcurrentDownloads { + if let resource = self.pendingResources.popFirst() { + self.startPrefetching(resource) } } } /// Stops current downloading progress, and cancel any future prefetching activity that might be occuring. public func stop() { - prefetchQueue.async { - if self.finished { return } - self.stopped = true - self.tasks.values.forEach { $0.cancel() } - } + if finished { return } + stopped = true + tasks.values.forEach { $0.cancel() } } func downloadAndCache(_ resource: Resource) { @@ -270,13 +259,11 @@ public class ImagePrefetcher { } func reportCompletionOrStartNext() { - prefetchQueue.async { - if let resource = self.pendingResources.popFirst() { - self.startPrefetching(resource) - } else { - guard self.tasks.isEmpty else { return } - self.handleComplete() - } + if let resource = self.pendingResources.popFirst() { + startPrefetching(resource) + } else { + guard tasks.isEmpty else { return } + handleComplete() } }