Clarity: Datagrid memory leak and DOM elements leak.

[fleshka779]

[x] bug
[ ] feature request
[ ] enhancement

Expected behavior

Datagrid not to leak memory and DOM elements.

Actual behavior

After constantly updating the table using a socket, memory leaks and element leaks are created.

Reproduction of behavior

• https://stackblitz.com/edit/angular-4-5-clarity-datagrid-memoy-leak
• Turn on the performance tab in chrome for one minute and you'll be surprised by the error
• https://monosnap.com/file/nz5FJKsxRrx7f5SVdH5xr2UaY6PCD8

Environment details

• Angular version: 4.4.5

• Clarity version: 0.10.23

• OS and version:

• Browser: [ Chrome 63.0.3239.132 ]

[srivathsah]

+1

[gnomeontherun]

I ran this stackblitz and profiled for 2.5 minutes. I see the memory gets GC'd several times over this span of time. Even in your screenshot, you see a GC event around 12s into the profile, but if you had let it run longer I would expect to see another GC event after about 45s according to my profiling, but that would obviously depend on your system. Based on this, I don't believe this is a Datagrid memory issue directly but perhaps an application issue. The Datagrid could make the memory issue more obvious due to the fact its our most complex, memory intensive component.

Now this doesn't have a websocket to update the datagrid, so perhaps there is something else your application is doing that is causing memory leaks that you could demonstrate?

[fleshka779]

@gnomeontherun thanks, we will investigate the problem in our app

[fleshka779]

@gnomeontherun @mathisscott Hi, I investigated this issue.
Problem was in *clrDgHideableColumn="{hidden: !property.visible}".
I'll add it to test project(https://stackblitz.com/edit/angular-4-5-clarity-datagrid-memoy-leak).
Please check it again.

ScreenShot: https://monosnap.com/file/F9lHR3AkLH0u4HYqSZ41EQg6PNo4DS
And you can reproduce it with:
"@clr/angular": "^0.11.0-rc.1", "@clr/icons": "^0.11.0-rc.1", "@clr/ui": "^0.11.0-rc.1",

Angular: ^5.1.3

[gnomeontherun]

Interesting, and yes now I see it happening. Quick investigation makes me think we've got a rogue observable not being cleaned up in the hidden columns. The memory profiler is pointing to a growing array of subscriptions being tracked, looking like its the row itself not cleaning up.

https://github.com/vmware/clarity/blob/master/src/clr-angular/data/datagrid/datagrid-row.ts#L207

[mbrookson]

@gnomeontherun I wonder if I'm seeing a similar issue with the Kendo library. I've seen other GitHub threads and StackOverflow articles about Observables in RxJs that say Angular and RxJs take care of not leaving Observables hanging around, so how do you know which ones need manually unsubscribing?
I'm seeing this same memory issue when the GC gets run but the memory usage keeps increasing over time

[gnomeontherun]

I investigated further, and don't believe that was the source. I addressed it but still saw the memory growing. There is more investigation required.

[youdz]

I started investigating this, and noticed you are reloading server-side objects without using trackBy. That's going to be a source of many, many problems, and is heavily discouraged. See https://angular.io/guide/template-syntax#ngfor-with-trackby for more information. Using trackBy will result in much better performances and increased consistency, as you can see in this fork of your StackBlitz: https://stackblitz.com/edit/angular-4-5-clarity-datagrid-memoy-leak-ssbper?file=app/app.component.html
If you check the performance again, there is no memory leak anymore.

*clrDgItems supports a trackBy option just like *ngFor does, and you should always use it when dealing with objects you receive from the server.

That said, it doesn't change the fact that we have a memory leak. It's just that the memory leak happens when a discouraged behavior is implemented, which makes it a much lower priority than we thought it was. I will check with the team if we still want to allocate time to this right now, and if we do I will continue investigating. Otherwise, we might push this bug further down the line and address it after higher priority ones.

[gnomeontherun]

Closing as resolved by the triage behind #4712.

[github-actions[bot]]

Hi there 👋, this is an automated message. To help Clarity keep track of discussions, we automatically lock closed issues after 14 days. Please look for another open issue or open a new issue with updated details and reference this one as necessary.