philippec / PhFacebook

MacOSX Interface to Facebook graph API
http://developer.casgrain.com/?p=107
Other
178 stars 44 forks source link

New thread created for every request #13

Closed vytis closed 13 years ago

vytis commented 13 years ago

When a request is made it is launched with [NSThread detachNewThreadSelector:] which in turn creates a new thread. This is not really efficient, as getting something like a profile picture of all your friends can quickly turn into a big mess.

I was thinking of changing the NSThread calls to use Grand Central Dispatch dispatch_async(). It is quite simple and means that all subsequent calls are queued nicely and no unnecessary threads are created. This however only works on OS X 10.6+. I thought I'll ask what are your thoughts about backwards compatibility.

If you do think that using blocks is a good idea, I would like to extend the project to include calls that use a supplied block instead of a callback. It makes the code much cleaner and easier to read. For example for the simple sendRequest call an equivalent would be created:

- (void) sendRequest:(NSString*) request setCompletionBlock:(void (^)(NSString *result)) block

So rewriting part of the sample application using the new call would look like:

- (IBAction) sendRequest: (id) sender
{
    [self.send_request setEnabled: NO];
    [fb sendRequest: request_text.stringValue setCompletionBlock:^(NSString *result) {
        [self.send_request setEnabled: YES];
        [self.result_text setString: [NSString stringWithFormat: @"Request: {%@}\n%@",request_text.stringValue , result]];
    }];
}

Using this syntax the code for handling request is at the same place where the call is made, rather than putting everything in the - (void) tokenResult: (NSDictionary*) result callback. It would also make the API a bit cleaner, since there is no need of having the NSDictionary as the result - all variables inside it are available in the block anyway.

Let me know what you think as I would love to contribute to this project a bit more.

philippec commented 13 years ago

GCD and blocks are great, but for now I would like to have this framework still be 10.5-compatible.

Furthermore, you may overestimate the (percieved) inefficiencies of NSThread. Do you have Shark traces to prove it?

Network latency is a much, much bigger issue in terms of latency.

I won't pretend that my design is the best or the most efficient, but I don't see efficiency being the primary driver here.

That being said, if one can write less code to do more, that's obviously a plus.

Thank you for taking the time to contribute to this project!

vytis commented 13 years ago

I haven't done much testing to base my assumption, but it just seemed a bit unexpected that every request needs a new thread. On the other hand it's not much an issue with a sane amount of requests. A batched request should be sent if there are more than a few requests anyway.