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

[FIX] retain-cycle memory leak issue solved. #24

Merged
merged 1 commit into from
Aug 30, 2018
Merged

[FIX] retain-cycle memory leak issue solved. #24

merged 1 commit into from
Aug 30, 2018

Conversation

boraseoksoon
Copy link
Contributor

@boraseoksoon boraseoksoon commented Aug 29, 2018

Some critical closure retain-cycle causing memory leak issues were fixed where scrollDelegate and textFieldTarget capture closure making strong reference.


To give background story a little in advance,
I am using my own customization based on reel-search for application that will be released V1.0 soon :) (named : 'All Dayz')
Thank you for the awesome repository and I am very glad to try pull request to this.

To explain the pull request,
I fixed some critical memory leak issues caused by closure memory retain cycle.
It can be very easily reproduced if instead of existing sample example using Single ViewController, we use root-navigation controller having ViewController1 and push ViewController2 having reel-search and back to VC1.
Using navigation controller, if we test push and pop current single ViewController example, memory is not released after pop. for example, it increases more than like 500MB after push and pop 3 times since data.txt is huge data set.
That means, this memory leak issue occurs in real project unavoidably except for the case where we only use single VC app having reel-search.

Below is the memory leak cases where I push and pop 3 times current reel-search example.

2018-08-30 00 32 53

FYI, Memory leak retain cycles cases are categorized as 2 group as below attached :
(1) TextFieldTarget, NotificationCallbackWrapper case
2018-08-29 23 33 55

(2) ScrollDelegate case
2018-08-29 23 11 25

I resolved these retain cycles and result is below with same test case above.
2018-08-30 00 44 42

I hope it helps, Thank you!

Some critical closure retain-cycle causing memory leak issues were fixed where scrollDelegate and textFieldTarget capture closure making strong reference.
@0ber 0ber merged commit 7ac671b into Ramotion:master Aug 30, 2018
@0ber
Copy link
Contributor

0ber commented Aug 30, 2018

good job, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants