phibr0 / obsidian-charts

Charts - Obsidian Plugin | Create editable, interactive and animated Charts in Obsidian via Chart.js
https://charts.phib.ro/
GNU Affero General Public License v3.0
592 stars 27 forks source link

Memory leak due to missing chart lifecycle management #27

Closed nothingislost closed 2 years ago

nothingislost commented 2 years ago

I've been looking into memory management quite a bit in my CodeMirror Options plugin to make sure my edit mode rendered widgets aren't causing memory leaks. In the process of tracking down various leaks, I noticed that the Charts plugin isn't unloading/destroying the charts it creates, in general.

See this line here where the new chart is created https://github.com/phibr0/obsidian-charts/blob/616c5730942b95e69234b7a6ae7cd8aa91a3943a/src/chartRenderer.ts#L163

This should probably be assigned to a variable and then .destroy() should be called on it when the component unloads.

If you want to test this, you can artificially increase the size of your rendered objects, so that they're easier to spot in the heap profile by adding:

    const allocSize = 1024 * 1024 * 10;
    el.memLeak = new Uint8Array(allocSize);

to

https://github.com/phibr0/obsidian-charts/blob/616c5730942b95e69234b7a6ae7cd8aa91a3943a/src/chartRenderer.ts#L165

This will add a 10MB array to your objects.

Next, go render some charts in preview mode, then close all active panes, and then go into devtools -> memory -> take heap snapshot. You should see 10MB arrays for each of the charts you rendered:

image

Additional reference: https://stackoverflow.com/questions/63977476/chartjs-memory-leak-garbage-collection-does-not-clean-chart-object-or-arrays-a

phibr0 commented 2 years ago

I see. I actually noticed some slowdown in the Chart Creator when doing a lot of rerenders. Do you have any Idea on how I know when the Chart can be destroy?

nothingislost commented 2 years ago

You'll want to start using the markdown processor ctx/context which is what manages the component lifecycle. See here: https://github.com/obsidianmd/obsidian-api/blob/763a243b4ec295c9c460560e9b227c8f18d8199b/obsidian.d.ts#L2304

The tasks plugin has a good example of using context here https://github.com/schemar/obsidian-tasks/blob/main/src/QueryRenderer.ts

They add a child here: https://github.com/schemar/obsidian-tasks/blob/89d6ff09d8c5001cf2e8ed56c4002dcb44a35149/src/QueryRenderer.ts#L38

And then define methods to be called on load/unload here: https://github.com/schemar/obsidian-tasks/blob/89d6ff09d8c5001cf2e8ed56c4002dcb44a35149/src/QueryRenderer.ts#L78

You don't have to worry about calling load() or unload() at the right times as Obsidian will do that part for you, you just need to define what needs to happen using onload and onunload.

phibr0 commented 2 years ago

Thank you! This should be fixed now, remember to update your CodeMirror options plugin to also pass the context to the renderer now