Add opt-in asyncCacheTypeCheck to move cache probe off caller thread by onevcat · Pull Request #2521 · onevcat/Kingfisher

onevcat · 2026-04-18T12:20:04Z

Summary

Adds KingfisherOptionsInfoItem.asyncCacheTypeCheck. When set, the cache-existence probe that precedes every retrieveImage call is dispatched onto the cache's I/O queue via imageCachedTypeAsync, instead of running stat syscalls synchronously on the caller thread.
retrieveImage still returns a DownloadTask synchronously. With the opt-in flag it returns a shell, probes the cache asynchronously, and on cache miss links the resulting session task onto the shell via DownloadTask.linkToTask(_:).
Default behavior is unchanged; callers that do not pass the new option continue to go through the existing synchronous probe path.
Extracts deliverTargetCacheHit / deliverOriginalCacheHit helpers so both sync and async paths share their cache-hit delivery logic.

Motivation

Reported in #2512: under disk pressure, ImageCache.imageCachedType → DiskStorage.Backend.isCached → fileManager.fileExists performs stat syscalls on whatever thread called setImage. When setImage runs from UIKit layout callbacks (tableView(_:cellForRowAt:), collectionView(_:cellForItemAt:)) these syscalls hang the main thread.

Usage

imageView.kf.setImage(with: url, options: [.asyncCacheTypeCheck])

Test plan

bundle exec fastlane test destination:"platform=iOS Simulator,name=iPhone 17" — 336 tests pass.
New cases in KingfisherManagerTests covering memory hit, disk hit, cache miss → download, onlyFromCache, originalCache fallback, and cancellation of the returned shell.

Fixes #2512

The cache-existence probe that runs before KingfisherManager decides between a cache read and a network download previously invoked `ImageCache.imageCachedType` synchronously on the caller thread. Under disk pressure, its `stat` syscalls can hang the main thread when `setImage` is called from UIKit layout callbacks such as `tableView(_:cellForRowAt:)`. Adds `KingfisherOptionsInfoItem.asyncCacheTypeCheck`. When enabled, `retrieveImage` returns a `DownloadTask` shell synchronously, dispatches the probe via `imageCachedTypeAsync` on the cache I/O queue, and on cache miss links the resulting session task onto the shell with `linkToTask(_:)`. Default behavior is unchanged for existing callers. Refactors `retrieveImageFromCache` to share `deliverTargetCacheHit` / `deliverOriginalCacheHit` helpers with the new async path. Fixes #2512

Copilot

Pull request overview

Adds an opt-in asyncCacheTypeCheck option to move the cache-existence probe (the “is this cached?” check) off the caller thread, addressing reported main-thread hangs under disk pressure by dispatching cache-type checks onto the cache I/O queue while keeping the API synchronous (returns a DownloadTask immediately).

Changes:

Introduces KingfisherOptionsInfoItem.asyncCacheTypeCheck and parses it into KingfisherParsedOptionsInfo.
Adds an async-probe retrieval path in KingfisherManager that returns a DownloadTask shell immediately and links it to the real task after the async cache probe.
Refactors cache-hit delivery logic into shared helper methods and adds new unit tests covering key cache/miss/cancellation scenarios.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File	Description
`Sources/General/KingfisherOptionsInfo.swift`	Adds the new public option, docs, and parsed-options plumbing.
`Sources/General/KingfisherManager.swift`	Implements async cache-type probing path + shared cache-hit delivery helpers.
`Tests/KingfisherTests/KingfisherManagerTests.swift`	Adds tests for the new option across cache states, only-from-cache behavior, original-cache fallback, and cancellation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-18T16:57:12Z

@@ -760,99 +772,272 @@ public class KingfisherManager: @unchecked Sendable {
        let originalImageCacheType = originalCache.imageCachedType(
            forKey: key, processorIdentifier: DefaultImageProcessor.default.identifier)


Same as above for the original-cache probe: originalCache.imageCachedType(...) ignores options.forcedExtension, which can mis-detect cached originals when a forced extension is in use. Passing forcedExtension: options.forcedExtension would also align the sync and async probe paths.

Suggested change

forKey: key, processorIdentifier: DefaultImageProcessor.default.identifier)

forKey: key,

processorIdentifier: DefaultImageProcessor.default.identifier,

forcedExtension: options.forcedExtension)

Copilot · 2026-04-18T16:57:13Z

+
+        if options.onlyFromCache {
+            let error = KingfisherError.cacheError(reason: .imageNotExisting(key: source.cacheKey))
+            completionHandler?(.failure(error))


The public doc for retrieveImage says completionHandler is invoked on options.callbackQueue, but this onlyFromCache early-exit calls completionHandler directly on the caller thread. This is also now inconsistent with the new asyncCacheTypeCheck path (which dispatches onto callbackQueue). Consider wrapping this failure in options.callbackQueue.execute { ... } to match the documented behavior.

Suggested change

completionHandler?(.failure(error))

options.callbackQueue.execute {

completionHandler?(.failure(error))

}

Copilot · 2026-04-18T16:57:13Z

+            let wrapped = self.loadAndCacheImage(
                source: source,
                context: context,
-                completionHandler: completionHandler)?.value
+                completionHandler: completionHandler)
+            if let realTask = wrapped?.value {
+                shell.linkToTask(realTask)
+            }
        }


In the async probe path, shell is only linked when loadAndCacheImage(...) returns a non-nil wrapped task with a non-nil value. This misses at least two real cases: (1) provider-backed sources (.dataProviding) where cancellation relies on providerTask (not sessionTask/cancelToken), and (2) network loads that return nil due to async requestModifier initialization (where users are told to use onDownloadTaskStarted). In these cases the returned shell may never become cancellable. Consider handling .dataProviding by transferring the underlying providerTask onto shell, and in the async-request-modifier case wiring onDownloadTaskStarted (or otherwise capturing the downloader task) to link it back to shell.

Copilot · 2026-04-18T16:57:13Z

+    /// By default, ``KingfisherManager`` calls ``ImageCache/imageCachedType(forKey:processorIdentifier:forcedExtension:)``
+    /// before deciding between a cache read and a network download. That call performs file-system `stat` syscalls on
+    /// whatever thread invoked `setImage`. When `setImage` is called from UIKit layout callbacks such as
+    /// `tableView(_:cellForRowAt:)` or `collectionView(_:cellForItemAt:)` on a device under disk pressure, those syscalls
+    /// can hang the main thread.
+    ///
+    /// Opt in to this flag to move the probe onto the cache's I/O queue via
+    /// ``ImageCache/imageCachedTypeAsync(forKey:processorIdentifier:forcedExtension:callbackQueue:completionHandler:)``.
+    /// The ``DownloadTask`` returned from `setImage` is still delivered synchronously; if the probe discovers a cache
+    /// miss, the resulting network task is linked to the returned shell via ``DownloadTask/linkToTask(_:)``.
+    ///
+    /// - Note: If ``KingfisherOptionsInfoItem/loadDiskFileSynchronously`` is set, that option continues to govern the
+    ///   actual disk read. This flag only affects the existence probe that precedes the read.
+    case asyncCacheTypeCheck


The doc comment for asyncCacheTypeCheck states that KingfisherManager calls ImageCache.imageCachedType(forKey:processorIdentifier:forcedExtension:) by default, but the current sync probe in retrieveImageFromCache does not pass forcedExtension. Either update the manager probe to include forcedExtension (so the doc is accurate) or adjust this comment to reflect the actual behavior.

Copilot · 2026-04-18T16:57:13Z

+        manager.retrieveImage(with: url, options: [.asyncCacheTypeCheck]) { result in
+            XCTAssertNotNil(result.value?.image)
+            XCTAssertEqual(result.value!.cacheType, .none)
+
+        manager.retrieveImage(with: url, options: [.asyncCacheTypeCheck]) { result in
+            XCTAssertNotNil(result.value?.image)
+            XCTAssertEqual(result.value!.cacheType, .memory)
+
+        manager.cache.clearMemoryCache()
+        manager.retrieveImage(with: url, options: [.asyncCacheTypeCheck]) { result in
+            XCTAssertNotNil(result.value?.image)
+            XCTAssertEqual(result.value!.cacheType, .disk)
+
+        manager.cache.clearMemoryCache()
+        manager.cache.clearDiskCache {
+            manager.retrieveImage(with: url, options: [.asyncCacheTypeCheck]) { result in
+                XCTAssertNotNil(result.value?.image)
+                XCTAssertEqual(result.value!.cacheType, .none)
+                exp.fulfill()
+        }}}}}


testRetrieveImageWithAsyncCacheTypeCheckAcrossCacheStates is structured as deeply nested completion handlers with a trailing }}}}}}, which is hard to read and easy to break when editing. Consider refactoring to clearer sequencing (e.g., helper methods, chaining via a small state machine, or using async/await if available in the test target) so each step’s scope is explicit.

Suggested change

manager.retrieveImage(with: url, options: [.asyncCacheTypeCheck]) { result in

XCTAssertNotNil(result.value?.image)

XCTAssertEqual(result.value!.cacheType, .none)

manager.retrieveImage(with: url, options: [.asyncCacheTypeCheck]) { result in

XCTAssertNotNil(result.value?.image)

XCTAssertEqual(result.value!.cacheType, .memory)

manager.cache.clearMemoryCache()

manager.retrieveImage(with: url, options: [.asyncCacheTypeCheck]) { result in

XCTAssertNotNil(result.value?.image)

XCTAssertEqual(result.value!.cacheType, .disk)

manager.cache.clearMemoryCache()

manager.cache.clearDiskCache {

manager.retrieveImage(with: url, options: [.asyncCacheTypeCheck]) { result in

XCTAssertNotNil(result.value?.image)

XCTAssertEqual(result.value!.cacheType, .none)

exp.fulfill()

}}}}}

let expectedCacheTypes: [CacheType] = [.none, .memory, .disk, .none]

func runStep(_ index: Int) {

manager.retrieveImage(with: url, options: [.asyncCacheTypeCheck]) { result in

XCTAssertNotNil(result.value?.image)

XCTAssertEqual(result.value!.cacheType, expectedCacheTypes[index])

switch index {

case 0:

runStep(1)

case 1:

manager.cache.clearMemoryCache()

runStep(2)

case 2:

manager.cache.clearMemoryCache()

manager.cache.clearDiskCache {

runStep(3)

}

default:

exp.fulfill()

}

}

}

runStep(0)

Copilot · 2026-04-18T16:57:13Z

+        // Give the async cache probe time to resolve and link the real download task onto the shell,
+        // then cancel through the shell before flushing the delayed stub.
+        DispatchQueue.global().asyncAfter(deadline: .now() + 0.3) {
+            task?.cancel()
+            _ = stub.go()
+            teardownExp.fulfill()
+        }


testRetrieveImageWithAsyncCacheTypeCheckReturnsCancellableShellOnMiss uses a fixed asyncAfter(... + 0.3) delay to wait for the probe/link to happen before cancelling. This kind of timing-based coordination can be flaky on slow CI or under load. Prefer synchronizing on an explicit signal (e.g., an expectation fulfilled when the returned task becomes initialized/linked, or when the downloader’s stub is observed to start) before calling cancel().

Copilot · 2026-04-18T16:57:14Z

        let key = source.cacheKey
        let targetImageCached = targetCache.imageCachedType(
            forKey: key, processorIdentifier: options.processor.identifier)
-        
+


retrieveImageFromCache does not pass options.forcedExtension into imageCachedType(...). This can incorrectly report a cache miss/hit when .forcedCacheFileExtension is used, and it makes the default (sync) path behave differently from the new async probe path (which does pass forcedExtension). Consider calling imageCachedType(forKey:processorIdentifier:forcedExtension:) here with forcedExtension: options.forcedExtension.

onevcat requested a review from Copilot April 18, 2026 16:50

Copilot started reviewing on behalf of onevcat April 18, 2026 16:50 View session

Copilot AI reviewed Apr 18, 2026

View reviewed changes

onevcat added 8 commits April 19, 2026 11:35

Fix async cache probe shell cancellation

9fff0f9

Respect forced cache extension in sync probes

b01b38f

Honor callback queue for cache-only misses

bd644cf

Stabilize async cache probe cancellation test

26682f4

Refactor async cache state coverage test

104a8f4

Mark async cache state helper sendable

016c3ff

Stabilize processor cache retrieval tests

3d04428

Document async cache probe update

1224814

onevcat merged commit af9e2fc into master Apr 25, 2026
29 of 48 checks passed

onevcat deleted the fix/2512-async-cache-type-check branch April 25, 2026 07:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add opt-in asyncCacheTypeCheck to move cache probe off caller thread#2521

Add opt-in asyncCacheTypeCheck to move cache probe off caller thread#2521
onevcat merged 9 commits intomasterfrom
fix/2512-async-cache-type-check

onevcat commented Apr 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -760,99 +772,272 @@ public class KingfisherManager: @unchecked Sendable {
		let originalImageCacheType = originalCache.imageCachedType(
		forKey: key, processorIdentifier: DefaultImageProcessor.default.identifier)

Uh oh!

Conversation

onevcat commented Apr 18, 2026

Summary

Motivation

Usage

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants