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 when first using picker #73

Closed
Cez95 opened this issue Mar 30, 2018 · 13 comments
Closed

Memory Leak when first using picker #73

Cez95 opened this issue Mar 30, 2018 · 13 comments
Labels
Bug This is clearly a bug and will be fixed in priority

Comments

@Cez95
Copy link

Cez95 commented Mar 30, 2018

I have tested this multiple times and every device possible. Whenever I open my app fresh and grant the device permission to access my photos etc, the first time YP opens it creates a memory leak. Specifically on line 44 - 48, private let loadingContainerView: UIView = { let view = UIView() view.backgroundColor = UIColor(white: 0, alpha: 0.8) return view }()

I have tried pushing it to the main thread to no avail. Great pod btw.

@s4cha
Copy link
Member

s4cha commented Apr 2, 2018

@Cez95 Thanks for the report, I'm taking a look at the moment and let you know if I have any updates :)

@s4cha
Copy link
Member

s4cha commented Apr 2, 2018

Silly question in the meantime,
Are you using [weak picker] or [unowed picker] when using the api?

picker.didSelectImage = { [unowned picker] img in
    // image picked
   // code...
    picker.dismiss(animated: true, completion: nil)
}

@Cez95
Copy link
Author

Cez95 commented Apr 2, 2018 via email

@Cez95
Copy link
Author

Cez95 commented Apr 2, 2018 via email

@s4cha s4cha added the Bug This is clearly a bug and will be fixed in priority label Apr 2, 2018
@ghost
Copy link

ghost commented May 30, 2018

Hi guys,

Great lib - continue the good work because it's shaping to become one of the best imagePicker out there (and I've checked quite a few out)!

I wanted to also chime here as I am having a huge memory spike (60-70MB) as well but for me it happens when I apply any filter. It's not surprising as I can see the lib is loading a bunch of GC related frameworks (Metal, etc.) but they don't appear to ever get released from memory, even once the app enters background.

Would be great to be able to optimize the memory usage once the picker is dismissed so that the app doesn't get killed by the OS when in the background...

Thanks again for the hard work,
Francois

@NikKovIos
Copy link
Collaborator

Hello guys! For sure, the picker has memory leaks. I tested it with Xcode instruments, but not enough professional in them. If you could help us with eliminating these leaks, respect for you))

@s4cha
Copy link
Member

s4cha commented May 30, 2018

@fschaus First of all thanks a ton for the kind words 😊. We're trying our best to become to go-to solution for image picking 🚀 .
I am going to look at the memory usage and will report back.

Cheers,

@ghost
Copy link

ghost commented May 31, 2018

Thanks for the quick response guys! @NikKovIos I am also pretty bad at instruments (+ it keeps crashing on my app) so I can only use the visual memory debugger... I took a quick look at the code for the filter and there is no super obvious retain cycles there as far as I can tell.

Happy to continue digging a bit and reporting what objects I see retained in memory; that might then help you both get the intuition of what could be driving this.

Let me know if that would be helpful,
Francois

@ghost
Copy link

ghost commented May 31, 2018

Hey guys,

I updated to 3.1.1 and the memory mgmt is much improved - nicely done! A good and a bad news though - the bad is that the memory leak is still present when applying filter, the good is that I was able to identify the cases in which it happens vs. not by looking closely at the console.

Whenever I select any picture in my picture library, I get one of the two below messages:

  • 2018-05-31 16:11:24.695393-0700 Tap >[5233:1362840] [ImageManager] Unable to load image data, /var/mobile/Media/DCIM/102APPLE/IMG_2264.JPG

  • 2018-05-31 16:11:40.705606-0700 Tap >[5233:1362840] [ImageManager] Unable to load image data, /var/mobile/Media/DCIM/102APPLE/IMG_2263.HEIC

Tbh I don't know what class/ method prints these messages but I noticed that in the first case (.JPG) the memory works just fine once I load the filters - it peaks but gets released once the filter screen is dismissed. It's in the 2nd case when using the High Efficiency File Format (or at least when that error message appears) that the memory retention happens - peaking by 100-150MB and only getting ~30-50MB back.

Not sure if the imagepicker handles the two files differently (I guess it does given the vastly different memory outcomes) but that seems to be the culprit.

Hope this helps (a little bit at least)!
Best,
Francois

@s4cha
Copy link
Member

s4cha commented Jun 15, 2018

Hey Francois,

Thank you for taking the time to add details to this issue, this is very appreciated :)

I've tried to look for the possible leak this afternoon but can't. Actually the memory looks pretty sane on my end.

screen shot 2018-06-15 at 15 46 01

The spikes are each time I was applying filters but they come back to normal whenever I change view or close the picker. Which indicates that the memory is freed.

Cheers,

@ghost
Copy link

ghost commented Jun 15, 2018

Cool! If anything it might be corner case when using that specific type of picture (HEIC) that the leak appears. I would be happy to dig into this next week and report back if I can find anything specific...

To help me in that could you share with me:

  • Your hypothesis of where the print message comes from? It seems to be system message and not coming from a print statement in the code though...
  • Any clue where different formats would be handled differently in the code? Again, suspect that this will be handled by some Apple framework and not by your code, but if you have a sense of which method/ object/ API might be responsible for detecting file type/ apply conversion I might see if by mistake there is no 2 method/ object/ API being instantiated by mistake...

Any hints you can give me on where to focus first would be highly appreciated!

Thanks,
Francois

@ghost
Copy link

ghost commented Jul 2, 2018

Hey guys,

I owe you an update on this. It seems that this is now resolved. I did extensive profiling with my release build and don't see any memory traces/ leaks, except for an array of filters (but which is less than 8-16 bits) so all good! Feel free to close this! Thanks again for all the hard work!

@s4cha
Copy link
Member

s4cha commented Jul 3, 2018

@fschaus,
Please excuse the late reply,

Indeed the print message was generated and not from our code. Maybe the remaining memory issue was solved along with the log issue #81.

Thank very much for taking the time to post an update, this is very appreciated :)

Closing this for now, @Cez95 feel free to reopen if needed.

@s4cha s4cha closed this as completed Jul 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This is clearly a bug and will be fixed in priority
Projects
None yet
Development

No branches or pull requests

3 participants