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 in Line Chart data #2772

Open
barryloh opened this issue Sep 6, 2017 · 12 comments
Open

Memory leak in Line Chart data #2772

barryloh opened this issue Sep 6, 2017 · 12 comments
Assignees

Comments

@barryloh
Copy link

barryloh commented Sep 6, 2017

Memory Leak in Line Chart

When popping screen back or selecting another tab from the UITabBarController, there will be a memory leak that'll occur from the chart.

Attached are screenshots from Instruments. Please advise on how to proceed with solving this problem.

Thanks!

screen shot 2017-09-06 at 11 50 02 am

screen shot 2017-09-06 at 11 50 26 am

screen shot 2017-09-06 at 11 50 42 am

screen shot 2017-09-06 at 11 50 53 am

@JustLee-me
Copy link

I've had this problem, too 😰

@JustLee-me
Copy link

open func addDataSet(_ d: IChartDataSet!)
{
// Leaks Problem
_dataSets.append(d)
}

@liuxuan30
Copy link
Member

liuxuan30 commented Sep 13, 2017

when you pop back, do you see free is called at last? This seems a duplicated issue for me.
Is this reproducible on ChartsDemo? Mind to upload a instrument so I can take a look?

I would guess there is really nothing we can do if it's swift leak, as I don't see obvious mistake in the library code. But you are welcome to help debug as well.

#2425 possible dup. As #2425 said, sometimes instruments would report false leak. Just FYI I recently receive an email that my old radar is closed, and maybe fixed in Xcode 9. So we can try later as well.

@liuxuan30 liuxuan30 self-assigned this Sep 13, 2017
@DamirDjozicCoing
Copy link

I had same problem with memory leak, I had over 2000 memory leak problems with this (in many lines of code, all for this Charts pod) and my solution was this:

  1. Go to definition of HorizontalBarChartView

  2. there you will see, in init method that HorizontalBarChartRenderer is being called, so go to "HorizontalBarChartRenderer" definition

  3. go to "BarChartRenderer" definition from there

  4. This is the catch: make "dataProvider" variable weak
    e.g. @objc open weak var dataProvider: BarChartDataProvider?

  5. go back to HorizontalBarChartView

  6. go to definition of "XAxisRendererHorizontalBarChart"

  7. This is the catch: make "chart" variable weak also

This solved my problem, after this I didn't have memory leak anymore, anywhere

Hope this helps,
Cheers!

@JustLee-me
Copy link

JustLee-me commented Dec 7, 2017 via email

@liuxuan30
Copy link
Member

@DamirDjozicCoing I will check what you said. Thanks!

@liuxuan30
Copy link
Member

liuxuan30 commented Dec 8, 2017

@DamirDjozicCoing in our code base,

This is the catch: make "dataProvider" variable weak
e.g. @objc open weak var dataProvider: BarChartDataProvider?

This is already weak:
image

but for XAxisRendererHorizontalBarChart,
It indeed misses weak declaration. I will fix it later.
image

liuxuan30 added a commit to liuxuan30/Charts that referenced this issue Dec 8, 2017
@liuxuan30
Copy link
Member

liuxuan30 commented Dec 8, 2017

I'm not sure if #3078 can fix this issue; need @barryloh and other folks to test, as the original issue lies in legend from screenshot. needs to keep this open.

@DamirDjozicCoing
Copy link

Cool. My mistake about the first thing (that is already weak), I played with it so I probably thought that I put "weak" there also but I didn't. Anyways, it should be weak.

As for legend, I didn't use legend in my app, but I can check and see. Maybe it is the same problem like this but for some other variable.

@barryloh
Copy link
Author

barryloh commented Dec 8, 2017

Hey @liuxuan30 unfortunately I do not have access anymore of the source code that contains this issue.

@liuxuan30
Copy link
Member

Thanks for your help @DamirDjozicCoing !
if @barryloh cannot test or Damir does not find anything, I will close this.

@DamirDjozicCoing
Copy link

No problem, I'm glad I could help!! @liuxuan30

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

No branches or pull requests

4 participants