Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

New thread created for every request #13

Closed
wants to merge 2 commits into from

2 participants

Vytis Philippe Casgrain
Vytis
vytis commented June 02, 2011

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.

Philippe Casgrain
Owner

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
vytis commented June 03, 2011

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.

Vytis vytis closed this June 03, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 2 unique commits by 1 author.

Jun 02, 2011
Vytis A new thread is no longer created for every request, they are nicely …
…queued and

serviced sequentially.
182be26
Vytis Queue gets released as it should 546708e
This page is out of date. Refresh to see the latest.
1  classes/PhFacebook.h
@@ -19,6 +19,7 @@
19 19
     PhWebViewController *_webViewController;
20 20
     PhAuthenticationToken *_authToken;
21 21
     NSString *_permissions;
  22
+    dispatch_queue_t queue;
22 23
 }
23 24
 
24 25
 - (id) initWithApplicationID: (NSString*) appID delegate: (id) delegate;
81  classes/PhFacebook.m
@@ -11,6 +11,7 @@
11 11
 #import "PhAuthenticationToken.h"
12 12
 #import "PhFacebook_URLs.h"
13 13
 #import "Debug.h"
  14
+#import <dispatch/dispatch.h>
14 15
 
15 16
 #define kFBStoreAccessToken @"FBAStoreccessToken"
16 17
 #define kFBStoreTokenExpiry @"FBStoreTokenExpiry"
@@ -30,6 +31,8 @@ - (id) initWithApplicationID: (NSString*) appID delegate: (id) delegate
30 31
         _webViewController = nil;
31 32
         _authToken = nil;
32 33
         _permissions = nil;
  34
+        queue = dispatch_queue_create("com.phfacebook.requests", 0);
  35
+
33 36
     }
34 37
     DebugLog(@"Initialized with AppID '%@'", _appID);
35 38
 
@@ -41,6 +44,7 @@ - (void) dealloc
41 44
     [_appID release];
42 45
     [_webViewController release];
43 46
     [_authToken release];
  47
+    dispatch_release(queue);
44 48
     [super dealloc];
45 49
 }
46 50
 
@@ -145,52 +149,43 @@ - (void) setAccessToken: (NSString*) accessToken expires: (NSTimeInterval) token
145 149
 	[self notifyDelegateForToken: _authToken withError: errorReason];
146 150
 }
147 151
 
148  
-- (void) sendFacebookRequest: (NSDictionary*) allParams
  152
+- (void) sendRequest: (NSString*) request params: (NSDictionary*) params;
149 153
 {
150  
-    NSAutoreleasePool *pool = [NSAutoreleasePool new];
151  
-
152 154
     if (_authToken)
153  
-    {
154  
-        NSString *request = [allParams objectForKey: @"request"];
155  
-        NSString *str = [NSString stringWithFormat: kFBGraphApiURL, request, _authToken.authenticationToken];
156  
-        
157  
-        NSDictionary *params = [allParams objectForKey:@"params"];
158  
-        if (params != nil) 
159  
-        {
160  
-            NSMutableString *strWithParams = [NSMutableString stringWithString: str];
161  
-            for (NSString *p in [params allKeys]) 
162  
-                [strWithParams appendFormat: @"&%@=%@", p, [params objectForKey: p]];
163  
-            str = strWithParams;
164  
-        }
165  
-        
166  
-        NSURLRequest *req = [NSURLRequest requestWithURL: [NSURL URLWithString: str]];
167  
-        NSURLResponse *response = nil;
168  
-        NSError *error = nil;
169  
-        NSData *data = [NSURLConnection sendSynchronousRequest: req returningResponse: &response error: &error];
170  
-
171  
-        if ([_delegate respondsToSelector: @selector(requestResult:)])
172  
-        {
173  
-            NSString *str = [[NSString alloc] initWithBytesNoCopy: (void*)[data bytes] length: [data length] encoding:NSASCIIStringEncoding freeWhenDone: NO];
174  
-
175  
-            NSDictionary *result = [NSDictionary dictionaryWithObjectsAndKeys:
176  
-                str, @"result",
177  
-                request, @"request",
178  
-                self, @"sender",
179  
-                nil];
180  
-            [_delegate performSelectorOnMainThread:@selector(requestResult:) withObject: result waitUntilDone:YES];
181  
-            [str release];
182  
-        }
  155
+    {        
  156
+        dispatch_async(queue, ^{
  157
+
  158
+            NSString *str = [NSString stringWithFormat: kFBGraphApiURL, request, _authToken.authenticationToken];
  159
+            if (params != nil) 
  160
+            {
  161
+                NSMutableString *strWithParams = [NSMutableString stringWithString: str];
  162
+                for (NSString *p in [params allKeys]) 
  163
+                    [strWithParams appendFormat: @"&%@=%@", p, [params objectForKey: p]];
  164
+                str = strWithParams;
  165
+            }
  166
+            
  167
+            NSURLRequest *req = [NSURLRequest requestWithURL: [NSURL URLWithString: str]];
  168
+            NSURLResponse *response = nil;
  169
+            NSError *error = nil;
  170
+            NSData *data = [NSURLConnection sendSynchronousRequest: req returningResponse: &response error: &error];
  171
+            
  172
+            if ([_delegate respondsToSelector: @selector(requestResult:)])
  173
+            {
  174
+                NSString *str = [[NSString alloc] initWithBytesNoCopy: (void*)[data bytes] length: [data length] encoding:NSASCIIStringEncoding freeWhenDone: NO];
  175
+                
  176
+                NSDictionary *result = [NSDictionary dictionaryWithObjectsAndKeys:
  177
+                                        str, @"result",
  178
+                                        request, @"request",
  179
+                                        data, @"raw",                                    
  180
+                                        self, @"sender",
  181
+                                        nil];
  182
+                dispatch_async(dispatch_get_main_queue(), ^{
  183
+                    [_delegate requestResult:result];
  184
+                });
  185
+                [str release];
  186
+            }        
  187
+        });
183 188
     }
184  
-    [pool drain];
185  
-}
186  
-
187  
-- (void) sendRequest: (NSString*) request params: (NSDictionary*) params;
188  
-{
189  
-    NSMutableDictionary *allParams = [NSMutableDictionary dictionaryWithObject: request forKey: @"request"];
190  
-    if (params != nil)
191  
-        [allParams setObject: params forKey: @"params"];
192  
-
193  
-	[NSThread detachNewThreadSelector: @selector(sendFacebookRequest:) toTarget: self withObject: allParams];    
194 189
 }
195 190
 
196 191
 - (void) sendRequest: (NSString*) request
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.