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

Consider switching to a different iOS image loading library. #13

Open
DylanVann opened this issue Jun 20, 2017 · 10 comments
Open

Consider switching to a different iOS image loading library. #13

DylanVann opened this issue Jun 20, 2017 · 10 comments
Labels
feature A new feature to add. help wanted

Comments

@DylanVann
Copy link
Owner

DylanVann commented Jun 20, 2017

Although it's very performant and used in tons of applications the API for SDWebImage is sort of weird.

e.g. The API for prefetching:

Assign list of URLs to let SDWebImagePrefetcher to queue the prefetching, currently one image is downloaded at a time, and skips images for failed downloads and proceed to the next image in the list. Any previously-running prefetch operations are canceled.

completionBlock
block to be called when prefetching is completed first param is the number of completed (successful or not) requests, second parameter is the number of skipped requests

Weird things:

  • One at a time.
  • Cancels any previous prefetch operation.
  • Tells you the number of successful and unsuccessful requests, not which images were successful or unsuccessful.

e.g. The API for headers:

headersFilter
Set filter to pick headers for downloading image HTTP request.
This block will be invoked for each downloading image request, returned NSDictionary will be used as headers in corresponding HTTP request.

Weird things:

  • Headers are not a property of each request (they should be).
  • Why is a singleton property being used to dynamically determine the headers for each request?
  • What if I had two requests to the same url that should have different headers?
    (The api of headersFilter does not allow that)

Maybe there's another native library we could use that's more straightforward than this.

@faxioman
Copy link

My (not requested ;)) vote goes to Kingfisher.
It's a swift library with a great API and good memory management.

https://github.com/onevcat/Kingfisher

@StevenMasini
Copy link
Contributor

What's the problem with SDWebImage ? What are the pros and cons of each library ?
I think we need to answer these question before making a decision.

From my experience SDWebImage is a very good library to deal with image cache, it's also one of the oldest. Let's establish the pros and cons list of the most interesting solution first.

Also what would be the advantage of rewriting the component from scratch ?

@psugihara
Copy link

The original post seems to be about improving the situation for developers of FastImage.

As a potential FastImage user, the most important thing to me is performance. Wanted to share this performance comparison I ran across: https://github.com/kean/Image-Frameworks-Benchmark

nuke looks pretty impressive

@faxioman
Copy link

@StevenMasini as noted by @psugihara some (important) cons of sdwebimage are reported in the original post.

@psugihara I didn't know nuke... It seems great! Is it possible for you to run your benchmark using the version 5 of Kingfisher?

@psugihara
Copy link

@faxioman not my benchmark, just a user there too :)

For clarification, I was noting that the cons in the OP are not at very compelling to me because as a user I'm completely abstracted from the underlying APIs. Performance + the API of FastImage itself are my concerns.

@StevenMasini
Copy link
Contributor

Don't worry, I am not against changing or anything. Don't misunderstand me, I like to keep my mind open to alternatives. I just want to make sure this change worse it.

At least we have a good list of alternatives to consider now.

@DylanVann
Copy link
Owner Author

Sorry I don't respond to stuff as quickly as I could. My work is primarily not React Native at the moment and I haven't had a lot of time for this. I think over the next few weeks I may have more.

@StevenMasini These issues in SDWebImage do sometimes leak through into issues for FastImage, it's just that they don't come up that often.

e.g.

  • If you don't do prefetching then the prefetch issue will never come up for you.
  • If you don't try to do things with headers than the headers issue won't come up for you.

I think in the common use case SDWebImage definitely performs well, and maybe the solution is trying to contribute to making it better, rather than trying to switch to something else.

@psugihara Thanks for the link to that. So far all I've found is https://bpoplauschi.wordpress.com/2014/03/21/ios-image-caching-sdwebimage-vs-fastimage/ , which is probably outdated at this point, and I haven't had the time to look at benchmarking things myself.

@StevenMasini
Copy link
Contributor

@DylanVann Indeed I don't use neither prefetching nor headers. I wasn't aware of these issues.

But maybe they could be fixed using SDWebImage.
After if Nuke performance are what they claim to be, why not moving to Nuke.

@allthetime
Copy link

Hi, has there been any movement on this?

@dreampiggy
Copy link
Contributor

dreampiggy commented Mar 3, 2020

Although it's very performant and used in tons of applications the API for SDWebImage is sort of weird.

e.g. The API for prefetching:

Assign list of URLs to let SDWebImagePrefetcher to queue the prefetching, currently one image is downloaded at a time, and skips images for failed downloads and proceed to the next image in the list. Any previously-running prefetch operations are canceled.

completionBlock
block to be called when prefetching is completed first param is the number of completed (successful or not) requests, second parameter is the number of skipped requests

Weird things:

  • One at a time.
  • Cancels any previous prefetch operation.
  • Tells you the number of successful and unsuccessful requests, not which images were successful or unsuccessful.

e.g. The API for headers:

headersFilter
Set filter to pick headers for downloading image HTTP request.
This block will be invoked for each downloading image request, returned NSDictionary will be used as headers in corresponding HTTP request.

Weird things:

  • Headers are not a property of each request (they should be).
  • Why is a singleton property being used to dynamically determine the headers for each request?
  • What if I had two requests to the same url that should have different headers?
    (The api of headersFilter does not allow that)

Maybe there's another native library we could use that's more straightforward than this.

All this strange behavior already been fixed via SDWebImage 5.0, react-native-fast-image already upgraded.

And I’m the maintainer of this framework and found these old suck behavior really painful.

If you have some more feature request, feel free to open https://github.com/SDWebImage/SDWebImage/issues.

If it’s not hack or hard-coded to any business logic, I’ll always provide a solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature to add. help wanted
6 participants