ref(core): Wrap scopes in WeakRef when storing scopes on spans
#17712
Conversation
size-limit report 📦
|
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
There was a problem hiding this comment.
Interesting catch! Yeah, let's give this a try!
I wonder though how this lead to a memory leak. Sure, a span holds references to scopes but shouldn't the span be GC'd at some point, leading to the scopes also being GC'd? Or is there anything else holding a reference to any of the two that doesn't get GC'd?
This is just a shot in the dark basically. I investigated a heap snapshot and saw that we retained a lot of memory from isolationscopes so @mydea and I figured this might be worth a try. Could be the case if spans, for some reasons, are not ending. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
andreiborza
force-pushed
the
ab/weakref-scopes
branch
from
cb295d5 to
a9fc7c6
Compare
We often store scopes on spans via `setCapturedScopesOnSpan` and retrieve them via `getCapturedScopesOnSpan`. This change wraps the scopes in `WeakRef` to attempt fixing a potential memory leak when spans hold on to scopes indefinitely. The downside is scopes might end up with undefined scopes on them if the scope was garbage collected, we'll have to see if that's an issue or not.
andreiborza
force-pushed
the
ab/weakref-scopes
branch
from
a9fc7c6 to
251a3dd
Compare
We often store scopes on spans via
setCapturedScopesOnSpanand retrieve them viagetCapturedScopesOnSpan. This change wraps the scopes inWeakRefto attempt fixing a potential memory leak when spans hold on to scopes indefinitely.The downside is spans might end up with undefined scopes on them if the scope was garbage collected, we'll have to see if that's an issue or not.