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 memory leak when side scrolling LGV blocks #1540

Merged
merged 3 commits into from
Dec 11, 2020
Merged

Conversation

cmdcolin
Copy link
Collaborator

There was possibly a memory leak from side scrolling blocks. I saw memory increasing pretty high from sidescrolling lgv

I looked and this code was not called for example

diff --git a/plugins/linear-genome-view/src/BaseLinearDisplay/components/ServerSideRenderedContent.t>
index efd6e3600..66dd4de57 100644
--- a/plugins/linear-genome-view/src/BaseLinearDisplay/components/ServerSideRenderedContent.tsx
+++ b/plugins/linear-genome-view/src/BaseLinearDisplay/components/ServerSideRenderedContent.tsx
@@ -75,6 +75,7 @@ function ServerSideRenderedContent(props: { model: BlockModel }) {

     return () => {
       if (domNode && isHydrated) {
+        console.log('here')
         unmountComponentAtNode(domNode)
       }
     }
lines 1-12/12 (END)


We could adjust the use of the isHydrated variable but also it seems this fix on this branch is a simpler approach: always unmount because it is a do nothing if not mounted https://reactjs.org/docs/react-dom.html#unmountcomponentatnode

@cmdcolin
Copy link
Collaborator Author

Screenshot from 2020-12-09 22-52-03

Got up to 1gb here from sidescrolling. We get ~4gb per tab, and keeping memory low reduces gc activity, so keeps code faster (chrome will start aggressively gc'ing stuff to keep things under that limit, and the GC activates at much lower levels than the 4gb limit)

@cmdcolin
Copy link
Collaborator Author

might want to look at the 500mb worker allocation too...

@cmdcolin cmdcolin marked this pull request as ready for review December 11, 2020 17:50
@cmdcolin cmdcolin requested a review from rbuels December 11, 2020 17:50
@cmdcolin cmdcolin added the bug Something isn't working label Dec 11, 2020
@cmdcolin cmdcolin changed the title Fix memory leak blocks Fix memory leak when side scrolling LGV blocks Dec 11, 2020
@rbuels rbuels merged commit 2717a0f into master Dec 11, 2020
@rbuels rbuels deleted the fix_memory_leak_blocks branch December 11, 2020 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants