Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Coalesce same GETs into single request operation #499

Closed
aburgel opened this Issue Sep 4, 2012 · 8 comments

Comments

Projects
None yet
4 participants
Contributor

aburgel commented Sep 4, 2012

I'm developing an app which downloads lots of small images, some of which have the same url. This causes many duplicate requests (though some are caught by the http cache, if I'm lucky with the order of requests). This is wasteful, but I also would prefer to not keep track of this in my app, it feels like its something that should be handled by the networking layer.

Does anyone have a good solution to this problem (aside from using SDWebImage)? If not, I'd be happy to code something up if there was interest in adding it to the project.

I think the simplest solution to this problem is to wrap an image into a "Model". The Model has a property image, and encapsulates the request:

- (UIImage*) image {
    if (_image == nil) {
        AFImageRequestOperation* op = [[AFImageRequestOperation alloc] imageRequestOperationWithRequest: ...
        success:^(UIImage *image) {
            self.image = image;  // Observe via KVO
        }
        ...
        ];
        self.image = placeholderImage;
    }
    return _image;
}
Contributor

aburgel commented Sep 7, 2012

This might work but it would complicate my model layer considerably. I think this just reinforces the point that this problem should be handled by the networking layer. (Imagine if a web browser fired off a new request each time you had the same img in your web page. You could workaround this by some javascript magic, but that's just wrong.)

I've thought about some complicated ways of doing this (add multiple success/failure blocks to the operations and then merge operations with the same url when enqueuing). But then I read about the dependency management features of NSOperationQueue.

Perhaps when enqueuing, if you find an existing GET op with the same URL, you could just add the new one as a dependency of the existing. That would prevent the new op from running until the existing completes, and by that time, the response will be cached, so the new operation won't cause a network request.

Of course, this depends on your responses being cacheable, which in my case, they are (working with images). A more robust solution wouldn't depend on the cache.

I'm going to give this a shot and report back.

On 07.09.2012, at 16:28, Alex Burgel wrote:

This might work but it would complicate my model layer considerably. I think this just reinforces the point that this problem should be handled by the networking layer. (Imagine if a web browser fired off a new request each time you had the same in your web page. You could workaround this by some javascript magic, but that's just wrong.)

It's unfortunate that this complicates your model - but I think you underestimate the complexity of a solution that would work the way you are suggesting.
I've thought about some complicated ways of doing this (add multiple success/failure blocks to the operations and then merge operations with the same url when enqueuing). But then I read about the dependency management features of NSOperationQueue.

Perhaps when enqueuing, if you find an existing GET op with the same URL, you could just add the new one as a dependency of the existing. That would prevent the new op from running until the existing completes, and by that time, the response will be cached, so the new operation won't cause a network request.

The url of the GET is not sufficient. The response for the new request must be compatible with the response of the pending request, even regardless of the fact that the method is safe and idempotent. For example the Accept headers may be different. But then, forcibly serializing the requests might allow the subsequent requests to read from the cache - if there is one. But if the first operation is already queued, the chances are very high that it is already executing, and setting up a dependency has no effect then.

Of course, this depends on your responses being cacheable, which in my case, they are (working with images). A more robust solution wouldn't depend on the cache.

I'm going to give this a shot and report back.


Reply to this email directly or view it on GitHub.

Contributor

aburgel commented Sep 7, 2012

On Friday, September 7, 2012 at 12:14 PM, Andreas Grosam wrote:

The url of the GET is not sufficient. The response for the new request must be compatible with the response of the pending request, even regardless of the fact that the method is safe and idempotent. For example the Accept headers may be different. But then, forcibly serializing the requests might allow the subsequent requests to read from the cache - if there is one. But if the first operation is already queued, the chances are very high that it is already executing, and setting up a dependency has no effect then.

Of course, there is more to matching requests than just comparing URLs, but it would be easy to ignore requests that have set any additional headers or other properties, or even compare all headers and other properties to make a match.

I am not sure that its correct to say adding an already executing operation as a dependency has no effect. The docs for [NSOperation addDependency:] say adding a dependency to an already executing operation (the other way around) has no effect.

I have not tried it yet, so maybe you're right, but its not obvious from the docs that this is the case.

Ah, you are right for setting the dependency. So, it would be possible to serialize the subsequent "equal" requests utilizing a NSOperationQueue.

Still, there could possibly the problem that the "response handler" (say, the part of the code that deals with the NSData objects received in -connection:didReceiveData: delegate) of the response data of the first request is already executing. There is no guarantee that the original response data will be buffered completely through this "response handler" - unless you rely on the current implementation:

Currently, there is no such "response handler", unless we assign the NSInputStream object this role. This input stream will be used to hold the response data. An input stream though, is not required to buffer whole the data.

Contributor

mattt commented Sep 9, 2012

This is something that I encountered quite a bit when writing AFNetworking initially, while working on Gowalla. On an activity stream, it was likely to see the same set of faces or stamps in the first screen, which would result in duplicate requests.

Trying to solve this problem resulted in quite a few "too-clever-by-half" solutions. In the end, we determined that it actually wasn't worth it to try coalescing GETs—it was a premature optimization on something that didn't really have any significant impact on performance.

So my advice: don't worry about it—it's probably not as big of a deal as you think. And if it is, there may be ways to improve performance in the way you lay out the view.

@mattt mattt closed this Sep 9, 2012

Contributor

aburgel commented Sep 9, 2012

@mattt, thanks for the advice.

Contributor

3lvis commented Nov 17, 2014

Hey guys, I made a category that checks for in-progress GET requests before making new ones :)

https://github.com/NSElvis/AFHTTPSessionManager-AFUniqueGET

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment