0

I'm downloading multiple files like this

ALAssetsLibrary *library = [[ALAssetsLibrary alloc] init];

HUD = [[MBProgressHUD alloc] initWithWindow:self.view.window];
HUD.labelText = @"Downloading";
HUD.mode = MBProgressHUDModeIndeterminate;
[self.view.window addSubview:HUD];
[HUD show:YES];

dispatch_group_t group = dispatch_group_create();
dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH,0);

for (NSString *urlString in URLStrings) {
    dispatch_group_async(group, queue, ^{
        NSData *data = [NSData dataWithContentsOfURL:[NSURL URLWithString:urlString]];
        UIImage *image = [[UIImage alloc] initWithData:data];
        [library writeImageToSavedPhotosAlbum:[image CGImage] orientation:(ALAssetOrientation)[image imageOrientation] completionBlock:^(NSURL *assetURL, NSError *error) {


        }];
    });
}

dispatch_group_notify(group, queue, ^{
    dispatch_async(dispatch_get_main_queue(), ^{
        HUD.mode = MBProgressHUDModeText;
        HUD.labelText = @"Success";
        [self performSelector:@selector(hideHUDAndBack) withObject:Nil afterDelay:0.3];
    });
});

dispatch_release(group);

And it receives memory warning and shuts down when downloading files with total size of 20MB or more. I tried running it without gcd on the main thread but it still reveived memory warning at the end and shut down. What could be the main cause and how to fix it?

Dulguun Otgon
  • 1,925
  • 1
  • 19
  • 38

3 Answers3

2

First, you should monitor memory usage in the Instruments to find out the specific reason.

One thing that could help is including an @autorelease pool in the block, see for example Using ARC, is it fatal not to have an autorelease pool for every thread?.

But most importantly, good things happen automatically when you use higher-level frameworks, such as NSOperation class (built on top of GCD) or AFNetworking library (itself built on top of NSOperation).

They will create autorelease pools and provide a way to limit number of concurrent downloads, add dependencies, and do other stuff that you won't have to reimplement.

Note also that synchronous methods, like aforementioned dataWithContentsOfURL:, are likely to be less reasonable about memory footprint, because while they block the thread you are not able to perform any memory management.

Community
  • 1
  • 1
ilya n.
  • 18,398
  • 15
  • 71
  • 89
2

In your approach I don't see any advantage using dispatch lib in order to parallelize network requests. What one can achieve with multiple concurrent network requests is reducing the effect of network latency. However, your simple approach simultaneously introduces memory issues.

In the given scenario, loading videos from a remote server, we can assume that the file size is quite large, and thus latency becomes a minor problem. The dominant factor would be bandwidth. But you cannot load videos faster when you load multiple at once when the bandwidth is the limiting factor.

Thus, I would suggest you try the following even more simple solution:

for (NSString *urlString in URLStrings) {
    @autoreleasepool {
        NSData *data = [NSData dataWithContentsOfURL:[NSURL URLWithString:urlString]];
        UIImage *image = [[UIImage alloc] initWithData:data];
        [library writeImageToSavedPhotosAlbum:[image CGImage] orientation:(ALAssetOrientation)[image imageOrientation] completionBlock:^(NSURL *assetURL, NSError *error) {
            // HUD.infoText = @"Saved asset %@, assetURL";
        }];
    }
}
HUD.mode = MBProgressHUDModeText;
HUD.labelText = @"Download complete";
[self performSelector:@selector(hideHUDAndBack) withObject:Nil afterDelay:0.3];

Note: Since downloading the resources is sequential and synchronous, you should wrap these statements into a block and use dispatch_async, in order to execute it on a secondary thread (that is, not on the main thread).

What you now should improve is the way you download the videos. Method dataWithContentsOfURL: is the most inappropriate for loading remote resources. ;)

CouchDeveloper
  • 18,174
  • 3
  • 45
  • 67
  • The bandwidth thing is not completely correct. If the URLs point to different physical servers, an individual download may not use all the bandwidth available to the device. Anyway, I'd be tempted to put the content of the loop in an `@autoreleasepool { ... }` block. – JeremyP Nov 27 '13 at 10:26
  • Yes! even a single server may be able to effectively process several requests simultaneously (e.g. what if the speed is limited by video encoding which can only use single core?) So I recommend to *parallelize network requests*, but indeed not using dispatch lib. – ilya n. Nov 27 '13 at 11:01
  • I tried using AFNetworking it still caused memory warning and shut down. Have included the code in the update. – Dulguun Otgon Nov 27 '13 at 11:43
  • @JeremyP The maximum throughput over Wifi for an iPhone is about 150Mbps - which results in less than 12 Mbyte per second. Tell me a server which cannot saturate this bandwidth when *downloading* a file. LTE can be faster, but then the SSD of the device might become the limiting factor first, before any reasonable server chokes. Also, ADLS can be the limiting factor as well. – CouchDeveloper Nov 27 '13 at 12:43
  • @JeremP using an autorelease pool within the loop is definitely an improvement, Thanks for the hint! – CouchDeveloper Nov 27 '13 at 12:46
  • @CouchDeveloper I wasn't getting 12 Mbytes/sec out of Apple.com when I downloaded OS X Mavericks 10 minutes after it was made available :) – JeremyP Nov 27 '13 at 16:45
  • @JeremyP My ISP for example is much slower than 150Mbps, so I don't experience such setbacks from servers. :/ But if you really won't to squeeze the optimum performance in all scenarios with a scalable approach and moderate and constant memory foot-print on the device, you need a much, much more sophisticated solution (don't even think of AFN here!). More like a feedback loop controlled system which adapts to its environment. – CouchDeveloper Nov 27 '13 at 16:58
  • Would specifying outputstream to AFN decrease memory usage? – Dulguun Otgon Nov 28 '13 at 06:17
  • @DulguunLst Using AFN or NSURLConnection/NSURLSession which writes the received NSData chunks directly to a tmp file would reduce the memory foot-print. Then one could try to send multiple requests to the server. Loading the images from the file and writing them to the library should then be performed sequentially. – CouchDeveloper Nov 28 '13 at 08:06
  • Ok I think I've got rid of memory problem by directing output to `Cache`. Next when I try to move those pictures into camera roll by using `[library writeImageToSavedPhotosAlbum: orientation: completionBlock:]` it caused memory spike again. To make it save one image at a time I tried doing it recursively in the completion block and using dispatch_semaphore. But both method failed. – Dulguun Otgon Nov 28 '13 at 08:13
  • @DulguunLst I would download the images as files to a tmp directory. Each time a file is finished, dispatch a block to a serial queue (passing it the file path) which loads the image and writes it to the library. Note: in "theory" writing the image to the library should not require much memory. However, I don't know the internals of `writeImageToSavedPhotosAlbum`. If it turns out this uses a lot memory, you may *try* to alleviate the problem by loading the image through using a CGDataProvider (see CGDataProviderRef in the official docs). – CouchDeveloper Nov 28 '13 at 08:24
1

By the looks of it, you need to limit the number of concurrent downloads.
Credits should go here: https://stackoverflow.com/a/16105827/727817

Applied to your case it should look something like:

dispatch_group_t group = dispatch_group_create();
dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH,0);

dispatch_semaphore_t downloadSema = dispatch_semaphore_create(3);  // number of concurrent downloads - this should be set depending on the size of your images

for (NSString *urlString in URLStrings) {
    dispatch_group_async(group, queue, ^{
        dispatch_semaphore_wait(downloadSema, DISPATCH_TIME_FOREVER);
        NSData *data = [NSData dataWithContentsOfURL:[NSURL URLWithString:urlString]];
        UIImage *image = [[UIImage alloc] initWithData:data];
        [library writeImageToSavedPhotosAlbum:[image CGImage] orientation:(ALAssetOrientation)[image imageOrientation] completionBlock:^(NSURL *assetURL, NSError *error) {

            dispatch_semaphore_signal(downloadSema);
        }];
    });
}

// ..
dispatch_release(downloadSema);
Community
  • 1
  • 1
alex-i
  • 5,406
  • 2
  • 36
  • 56