Closed codemile closed 8 months ago
I converted this PR to a draft, based on feedback from @abrenneke the constructors
dependency is not stable and needs to be handled within the hook.
In our discussion, Andy mentioned there might be an atom already we can use or we need to convert getNodeConstructors()
call into an atom. Either way, placing this function call inside the hook triggered endless re-rendering of the context menu.
I looked, there wasn't an atom for node constructors, but I think one could be made really easily.
Fixed by e90d292c6013fa7be5aa8015519b5a805b004cfd
Closes #278
Summary
There is a memory leak in the app where the
WebView2
would hit 2gb of Ram and trigger the "This page is having a problem". I found references toWebView2
having memory leaks in Tauri via Google searches, and on Windows platform Tauri is using Microsoft Edge as it's WebView. I couldn't confirm if Mac users were experiencing this bug or not. I was able to reproduce the problem on Windows by opening multiple projects and leaving the app running.Here we can see memory increasing at a rate of 1mb/s while the app is idle.
https://github.com/Ironclad/rivet/assets/50146659/47ba62dc-a23a-417e-a246-dfdd524b58e0
I started debugging the leak by removing large chunks of code from the Rivet app until the leaks stopped. This process revealed the leaks to be coming from inside
<NodeCanvas>
component. There I narrowed the problem down to the<ContextMenu>
.I concluded by assumption that
@floating-ui/react
is the source of the memory leaks, and found these refs to people discussing memory leaks with the library. We use this library for positioning the popup context menu, and in addition wrap the<ContextMenu>
inside a<CssTransition>
which keeps the component in the DOM even when it's not visible.Memory Leak references:
https://github.com/floating-ui/floating-ui/issues/2576
https://github.com/floating-ui/floating-ui/pull/744
These things combined to create a problem, because
<ContextMenu>
was stuck in an endless render loop. Since<CssTransition>
keeps it in the DOM and@floating-ui/react
leaks memory with each render, then the render loop was causing the 1mb/s leak.Here we can see the render loop:
https://github.com/Ironclad/rivet/assets/50146659/c157362e-adf6-44eb-a16a-25b63d362389
I traced the render loop down to the
useContextMenuAddNodeConfiguration()
hook where it usesuseAsyncEffect()
to get theuiData
for the menu. The async hook would callsetUiData()
when done, this would trigger another render and the hook would fetch all the uiData again and callsetUiData
again in an endless loop.The fix ended up being the use of a dependency array on the hook. Once applied the memory leaks stopped, and I also moved the
constructors
variable outside of the hook as it's an external dependency.