fix(platform-node): Remove memory leak #557
Conversation
|
LGTM, thanks! |
|
Rad, thank you! |
| // TODO(gdi2290): hide cache in (ngPreboot|UniversalPreboot) | ||
| let prebootConfig = null; | ||
| let key = (typeof preboot === 'object') && JSON.stringify(preboot) || null; | ||
| let key = (typeof preboot === 'object') && preboot || null; |
There was a problem hiding this comment.
so the goal of this cache was reusing the same prebootEl but I guess the biggest problem was probably not detaching it from the dom after destroying the app. If we move preboot to createEngine() which is the plan then this is fine since we're maintaining the same reference to the preboot config. During each req/res we're creating a new preboot config which was why I wanted to ensure if the configs are the same that we reuse the preboot element
There was a problem hiding this comment.
ah, I was curious if that was why this was being stringified! Makes sense, and I frankly would've left it but WeakMaps don't support non-object keys so to get the memory releasing map it has to be the original object
Until moving to createEngine(), could also consider using an LRU or LFU cache in that case if you want to have serialized keys in this cache without risks of memory leaks. happy to send a followup PR if there is any other way you want this implemented here 👍
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes a memory leak
If you run servers in production, NgModules are generated per request and cached indefinitely, leading to a serious memory leak that will eventually floor your servers and you'll have to restart them constantly (up to every 15 minutes!)
Use WeakMap instead of Map to ensure if all references are lost to a cache key the value is garbage collected
No