Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory Leak #189

Closed
chadwilken opened this issue Dec 3, 2015 · 6 comments
Closed

Memory Leak #189

chadwilken opened this issue Dec 3, 2015 · 6 comments

Comments

@chadwilken
Copy link

I am using a pretty vanilla setup of the library and simply presenting the view controller and scrolling to the top adds ~40mb to my in use memory. Have you experienced this before and if so do you have a work-around?

[PHPhotoLibrary requestAuthorization:^(PHAuthorizationStatus status){
    dispatch_async(dispatch_get_main_queue(), ^{

        CTAssetsPickerController *picker = [[CTAssetsPickerController alloc] init];
        PHFetchOptions *options = [[PHFetchOptions alloc] init];

        options.predicate = [NSPredicate predicateWithFormat:@"mediaType == %d", PHAssetMediaTypeImage];
        picker.assetsFetchOptions = options;
        picker.delegate = self;
        picker.showsCancelButton = YES;

        [self presentViewController:picker animated:YES completion:nil];
    });
}];

// The delegate method
- (void)assetsPickerController:(CTAssetsPickerController *)picker didFinishPickingAssets:(NSArray *)assets
{
    self.uploadAssets = [assets mutableCopy];

    [self dismissViewControllerAnimated:YES completion:nil];

    dispatch_async(dispatch_get_main_queue(), ^{
        self.uploadingImageCount = self.uploadAssets.count;
        self.completedUploadImageCount = 0;

        self.alertView = [[SCLAlertView alloc] init];
        self.alertView.customViewColor = [UIColor ccBlueColor];

        [self.alertView showWaiting:self.tabBarController title:@"Uploading Photos" subTitle:@"Please wait while we upload your photos" closeButtonTitle:nil duration:0];

        // This recurses and uploads to my server, even upon completion it still maintains a large in memory footprint.
        [self fetchImageForAsset:[self.uploadAssets firstObject]];
    });
}
@chrisze
Copy link
Contributor

chrisze commented Dec 4, 2015

@chiunam : First of, thank you for creating this pod. It looks great. We've just integrated it with our app and are testing it to learn more about it. I also did notice that it uses a lot of memory when scrolling through a large collection of images. On iPhone 6 Plus with 2000 images, the working set starts at 30 MB when the asset picker is first presented, and goes up to almost 200 MB when I scroll through the collection a few times.

After a cursory look, it doesn't seem like a memory leak though - eventually, the memory gets cleaned up if I dismiss the picker and present it again. Still, that's a lot of memory. Possibly related to that - scroll performance seems to be an issue on iPhone 6 Plus. I agree with your comment earlier that perf is adequate on iPhone 6, but with more images visible on screen + higher resolution on iPhone 6 Plus, the experience is not smooth.

I've spend less than an hour looking at the code so far, but it feels like PHCachingImageManager is contributing to both problems. In CTAssetsGridViewController, if I replace:

- (void)scrollViewDidScroll:(UIScrollView *)scrollView
{
    [self updateCachedAssetImages];
}

with

- (void)scrollViewDidEndDecelerating:(UIScrollView *)scrollView
{
    [self updateCachedAssetImages];
}

then scrolling is a lot smoother, and the app ends up using a lot less memory (I couldn't go past 60 MB in the exact same experiment as above).

/cc: @chadwilken

@chadwilken
Copy link
Author

Good find! I would love to get a patch of this soon. If you need help with anything let me know.

@chrisze
Copy link
Contributor

chrisze commented Dec 5, 2015

I've updated my PR to include the fix for this: #188. I changed thumbnail request resize mode from PHImageRequestOptionsResizeModeExact to PHImageRequestOptionsResizeModeFast, which seems to address the memory usage problem and improve scroll performance. Having said that, I think it would be good to test and iterate on it some more. When playing with the picker a bit more extensively, the grid view froze; when I attached the debugger, there were more than 100 photo library worker threads. I doubt that's related to resize mode, since I've seen similar behavior before.

@1and2papa
Copy link
Owner

@chrisze Thanks a a lot for your investigation and contribution. I need some time to study and test your PR. Might not able to merge it in a short time as I am pretty busy recently.

@chadwilken Please help to test @chrisze's PR if you are free.

@chadwilken
Copy link
Author

I did some testing of the PR and it seems to be much better. I didn't get any deadlocks and the memory issues got a tad better as well.
cc/ @chrisze @chiunam

@1and2papa
Copy link
Owner

Guys, sorry for being late. I'm going to merge his PR. Just have to clarify some changes with @chrisze. Stay tuned.

1and2papa added a commit that referenced this issue Jan 11, 2016
#189 Fix memory issue and visual bugs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants