Declare explicit objectWillChange publishers in ImageManager and IndicatorStatus by sboko83 · Pull Request #366 · SDWebImage/SDWebImageSwiftUI

sboko83 · 2026-10-09T09:59:54Z

Problem

ImageManager, IndicatorStatus and WebImageConfiguration have no @Published
property and rely on the synthesized objectWillChange.
The synthesized getter resolves the publisher through a global lookup on every
access, so each send() during image loading pays that cost. In a screen with
many WebImage views updating at once this shows up as main-thread time
dominated by the publisher lookup.

Fix

Declare a stored objectWillChange = ObservableObjectPublisher() in all three
classes (public in the two public ones). WebImage.swift gets import Combine,
which ObservableObjectPublisher needs. Behaviour is unchanged: same publisher
type, same send sites; WebImageConfiguration never sends.

Impact

In our app (a scrolling list of WebImage views plus a sheet with artwork),
main-thread time for the same interaction dropped from ~1110 ms to ~427 ms.

@topkim993 measured a screen with a few hundred WebImage views: main-thread CPU
for the same scroll dropped by ~30% with ImageManager and IndicatorStatus, and
by just under 40% with WebImageConfiguration added
(comment). On our smaller screens
the table lookup is already gone after the first two classes, so the third one is
neutral there — no regression either.

Independent of #364: that PR changes where send() is called, this one only
changes how the publisher is stored.

Summary by CodeRabbit

Bug Fixes
- Improved change notifications across image loading, indicator status, and web image configuration, helping SwiftUI views stay in sync when these observable values update. This can make UI refresh behavior more consistent as image-related state changes.

- ImageManager and IndicatorStatus get a stored ObservableObjectPublisher - the synthesized getter scans a global weak table on every send(), O(N) per image event

coderabbitai · 2026-10-09T10:00:11Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: b08b6048-043f-4ca2-88ce-21db4bd4591e

📥 Commits

Reviewing files that changed from the base of the PR and between d7fbe2f and 81fd055.

📒 Files selected for processing: 1

SDWebImageSwiftUI/Classes/WebImage.swift

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

ImageManager, IndicatorStatus, and WebImageConfiguration now declare explicit ObservableObjectPublisher properties named objectWillChange. WebImage.swift imports Combine.

Changes: ObservableObject Publishers

Layer / File(s)	Summary
Explicit publisher declarations `SDWebImageSwiftUI/Classes/ImageManager.swift`, `SDWebImageSwiftUI/Classes/Indicator/Indicator.swift`, `SDWebImageSwiftUI/Classes/WebImage.swift`	`ImageManager`, `IndicatorStatus`, and `WebImageConfiguration` now declare explicit `objectWillChange` publishers. `WebImage.swift` imports Combine.

Priority: ➖ Normal

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 81fd0

No actionable merge-blocking risk remains.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title names the two main classes changed, ImageManager and IndicatorStatus, and the change: declaring explicit objectWillChange publishers instead of relying on the synthesized one. It matches the…
Docstring Coverage		No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.

Full details: Title check

Explanation

The title names the two main classes changed, ImageManager and IndicatorStatus, and the change: declaring explicit objectWillChange publishers instead of relying on the synthesized one. It matches the core of the PR objectives. It omits the WebImageConfiguration change in WebImage.swift, which the later commits add, but that is a secondary detail and the title does not need to list every file.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3 files.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

sboko83 · 2026-10-09T10:14:49Z

The Unit Test failures are unrelated to this change: the only failing test is WebImageTests.testWebImageWithAnimatedURL on all three platforms, because apng.onevcat.com no longer resolves (NSURLErrorDomain -1003). All other tests, Build Library, Cocoapods Lint and Run Demo pass. #367 replaces the dead URL; once it is merged I will rebase this branch to get a green run.

topkim993 · 2026-10-22T07:21:34Z

Just want to add a data point. We ran into the same thing while profiling a screen with a lot of WebImage instances alive at once (a few hundred as you scroll through). Almost everything on the main thread was objectWillChange.getter → ObservableObjectTable.collect(), which was a surprise until we realized these classes have no @Published property, so Combine falls back to that global table lookup.

I tried this PR as is and it helped a lot: main thread CPU for the same scroll dropped by roughly 30%, and nothing else changed behaviour-wise.

One thing I noticed afterwards: WebImageConfiguration in WebImage.swift is in the same situation (no @Published) and still shows up in the profile after this change. Adding the same let objectWillChange = ObservableObjectPublisher() there took the total reduction to just under 40%. Might be worth folding into this PR while you're at it.

Thanks for putting this up, hope it gets merged soon.

@published

- WebImageConfiguration has no @published either, so its synthesized objectWillChange also goes through the global ObservableObjectTable - add import Combine: ObservableObjectPublisher is not visible via SwiftUI

sboko83 · 2026-10-23T11:53:13Z

Thanks for the data point and for the extra class — added WebImageConfiguration in 81fd055. It also needs import Combine in WebImage.swift: ObservableObjectPublisher is not visible through SwiftUI alone.

For the record, on our side (iPhone 11, Release, scroll of a home screen with fewer WebImage views) the profile shows 0 ObservableObjectTable samples already after the first two classes, so the third one measures as neutral there (338 → 331 ms/s main thread, within noise). Your screen with a few hundred views is exactly where it should show up, so I've noted your numbers in the description.

Declare explicit objectWillChange publishers

d7fbe2f

- ImageManager and IndicatorStatus get a stored ObservableObjectPublisher - the synthesized getter scans a global weak table on every send(), O(N) per image event

sboko83 mentioned this pull request Oct 9, 2026

Fix testWebImageWithAnimatedURL: replace dead APNG host #367

Open

Declare explicit publisher in WebImageConfiguration

81fd055

- WebImageConfiguration has no @published either, so its synthesized objectWillChange also goes through the global ObservableObjectTable - add import Combine: ObservableObjectPublisher is not visible via SwiftUI

Navigation Menu

Repository navigation

Declare explicit objectWillChange publishers in ImageManager and IndicatorStatus - #366

Declare explicit objectWillChange publishers in ImageManager and IndicatorStatus#366
sboko83 wants to merge 2 commits into
SDWebImage:masterfrom
sboko83:fix/explicit-objectwillchange

sboko83 commented Oct 9, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 9, 2026 •

edited

Loading

Uh oh!

sboko83 commented Oct 9, 2026

Uh oh!

topkim993 commented Oct 22, 2026 •

edited

Loading

Uh oh!

sboko83 commented Oct 23, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

sboko83 commented Oct 9, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Impact

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sboko83 commented Oct 9, 2026

Uh oh!

topkim993 commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sboko83 commented Oct 23, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sboko83 commented Oct 9, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 9, 2026 •

edited

Loading

topkim993 commented Oct 22, 2026 •

edited

Loading