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

Reopen issue 7854 #7963

Closed
nikolastojanovictt opened this issue May 14, 2024 · 3 comments
Closed

Reopen issue 7854 #7963

nikolastojanovictt opened this issue May 14, 2024 · 3 comments
Labels

Comments

@nikolastojanovictt
Copy link

Issue: #7854

Is marked "DONE" with:
AG-7519 [React] Investigate potential memory leak in React after calling setRowData
in changelog of v31.3.1.

However, issue is still reproducible.

@AG-Zoheil
Copy link

Hi @nikolastojanovictt,

Thanks for following up on this. We are unable to see evidence of a memory leak in v31.3.2

See here for comparison:

v31.2. https://plnkr.co/edit/BMkZ7eAdb87z2luB?open=index.tsx&preview
v31.3.2 https://plnkr.co/edit/3OG0A8cnc5tSZiud?open=index.tsx

Can we please ask you to follow this sequence of steps to provide us with more information in regards to this:

  1. Either use the plunker provided above or one of your one
  2. Open console and navigate to memory tab
  3. Take several heap snapshots over a period of time to show that show potential memory leak
  4. Save the profile by downloading it from the console
  5. Please send us the file of snapshots showcasing the observed memory leak

This would be the best way forward for us to investigate this further.

Much appreciated!

Kind regards,
Zoheil

@nikolastojanovictt
Copy link
Author

Hi Zoheil,

Thank you for checking this issue out. I noticed three things about your Plunker links:

  1. both use 31.2.0 Ag Grid, although second one should use 31.3.2 if I understood you correctly
  2. none of them uses React version used in original issue Possible memory leaks of applyTransactionAsync #7854 (React 18.2.0)
  3. none of them treats issue mentioned in original issue Possible memory leaks of applyTransactionAsync #7854 - update of data with applyTransactionAsync

I analyzed memory with your Plunker examples (which use 31.2.0 AgGrid). Since it had memory leaks (measured about 6 minutes timespan), I created a fork of your code with updated versions (AgGrid 31.3.2 and react 18.2.0). After measuring memory in 10 minutes span, it seems that memory is managed well. So manipulating rowData via AgGrid component prop is probably OK.


Back to original issue, applyTransactionAsync, I created a fork of my previous Plunker, mentioned in #7854 : https://plnkr.co/edit/d3MII5h8YswsuN3N?open=index.tsx
It uses AgGrid v31.3.2 now. Issue with memory is still reproducible. Here are links to 4 memory snapshots, made in 8 minute span (files are ~70MB each):
https://drive.google.com/file/d/1R4JAf3wjo7IJUYQy0aZ86QoiEmT7KE4i/view?usp=sharing
https://drive.google.com/file/d/1gtE2lNu02sjPQKsdcaJvbU3cTEJ_Ru66/view?usp=sharing
https://drive.google.com/file/d/1ONTT_uZ0WhaH713LMKi542Dm3RT1TjFr/view?usp=sharing
https://drive.google.com/file/d/18l6cUm_akWT_TPLoS22W0vdU6Es9K6_l/view?usp=sharing

All measurements I've done were made by taking heap snapshots of "pnklr.co:Main" Javascript VM instance. During the time of measurement, I didn't navigate from selected browser tab, nor did I change viewed file from Plunker file section, located on the left. All browser extensions have been turned off.

@AG-Zoheil
Copy link

Hello @nikolastojanovictt,

Thank you for your thorough review and comment.

I have raised another item in our backlog to specifically look at the scenario you've described. We appreciate you forking and providing a most up to date plunker as well.

We have added this requirement to our backlog and we are tracking it with the following reference: AG-11669

We have no timeline for this at the moment, but this can still be picked up by our team when they're making modifications to this functional area of AG Grid. Please use the functionality currently provided by AG Grid until this is delivered at a later point.

See whether this item will be in the next release by checking the NEXT RELEASE checkbox on the product pipeline page:
https://www.ag-grid.com/ag-grid-pipeline/

The best way to track this is to sign up for AG Grid new release notifications using the instructions here. This way you'll know as soon as a new version is out and you can check whether this specific item was implemented on the changelog page.

Thanks again for bringing this up with us.

Kind regards,
Zoheil

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

No branches or pull requests

2 participants