Skip to content
This repository was archived by the owner on Jan 17, 2023. It is now read-only.

Improved Image Loading Performance By Inflating Image In Background#1070

Merged
mattt merged 3 commits into
masterfrom
experimental-background-image-inflation
Jun 18, 2013
Merged

Improved Image Loading Performance By Inflating Image In Background#1070
mattt merged 3 commits into
masterfrom
experimental-background-image-inflation

Conversation

@mattt

@mattt mattt commented Jun 17, 2013

Copy link
Copy Markdown
Contributor

Following a hot tip from a couple developers at WWDC, I attempted to narrow the performance gap for image loading.

Currently, there is a noticeable stutter on some devices when settings images, because compressed image formats (such as PNG) are decompressed and set on the main thread in the callback.

In preliminary testing, this patch improves performance significantly—removing the aforementioned stutter completely. It does so in a couple of ways:

  • Creating an inflated / decompressed image in the responseImage getter, which is normally accessed in the background during the completionBlock
  • Staying close to the metal when allocating CGImageRef (i.e. using CGImageCreateWithPNGDataProvider or CGImageCreateWithJPEGDataProvider depending on the response headers of the operation).

I'm really excited by the initial success of this, but want to make sure I got everything right before merging this in (my Quartz-jitsu is a little rusty).

Any feedback? /cc @steipete @blakewatters @0xced

@steipete

Copy link
Copy Markdown

Looks good in general. I would actually apply this to the image category and not in the AFImageRequestOperation, or provide a setting to enable/disable this. There might be cases where you simply want to prefetch images and save them on disk, and this process of caching will destroy any metadata that might be in the JPG and is also quite memory/CPU intensive, not something we want for prefetching.

@mattt

mattt commented Jun 17, 2013

Copy link
Copy Markdown
Contributor Author

@steipete Thanks so much for lending your expert opinion to all of this.

Putting this on the UIImageView category is one possibility; putting it in AFImageRequestOperation (with a automaticallyInflatesResponseImage BOOL @property, probably) seemed the most convenient, as it would expose this functionality more broadly (which would be particularly useful for things like a UIButton+AFNetworking category).

I'll keep testing and iterating on this. For the most part, I'm downright thrilled to crack the nut on one of the remaining AFNetworking performance issues.

@steipete

Copy link
Copy Markdown

Right on, having an automaticallyInflatesResponseImage property that defaults to YES might be the best choice.

@steipete

Copy link
Copy Markdown

So I've talked to an (ex)Apple-Engineer about this. (thanks, @danielboedewadt !)

There's some information on allowed bitmap context combinations in https://developer.apple.com/library/mac/#documentation/graphicsimaging/conceptual/drawingwithquartz2d/dq_images/dq_images.html#//apple_ref/doc/uid/TP30001066-CH212-TPXREF101

We should use kCGBitmapByteOrder32Host which will map to kCGBitmapByteOrder32Little since ARM is (like x86) little endian, at least on the Apple platforms.

…peration

Falling back on initWithData:scale: initialization when available (iOS > 5)
@mattt

mattt commented Jun 18, 2013

Copy link
Copy Markdown
Contributor Author

We should use kCGBitmapByteOrder32Host which will map to kCGBitmapByteOrder32Little since ARM is (like x86) little endian, at least on the Apple platforms.

Adding that introduced a noticeable stutter, for whatever reason. Not sure if I was using it correctly, so pull requests welcome, if you think I could be doing this better.

With the new property in place, I'm confident in merging this in.

One less reason why people would want to use SDWebImage in 3... 2... 1...

mattt added a commit that referenced this pull request Jun 18, 2013
…age-inflation

Improved Image Loading Performance By Inflating Image In Background
@mattt mattt merged commit 5efa6a0 into master Jun 18, 2013
@mattt mattt deleted the experimental-background-image-inflation branch June 18, 2013 15:59
@bogardon

Copy link
Copy Markdown
Contributor

is there support for calling the routine AFInflatedImageFromResponseWithDataAtScale by itself?

Nevermind just found

- (id)responseObjectForResponse:(NSURLResponse *)response data:(NSData *)data error:(NSError *__autoreleasing *)error;

greghe pushed a commit to skillz/AFNetworking that referenced this pull request Sep 3, 2015
…ackground-image-inflation

Improved Image Loading Performance By Inflating Image In Background
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants