Skip to content

Fixed a memory leak in ItemMeasurer#239

Merged
bvaughn merged 1 commit into
bvaughn:issues/6from
clangen-nw:issues/6
May 27, 2019
Merged

Fixed a memory leak in ItemMeasurer#239
bvaughn merged 1 commit into
bvaughn:issues/6from
clangen-nw:issues/6

Conversation

@clangen-nw

Copy link
Copy Markdown

Noticed memory was ballooning over time in an app I'm working on; fired up the Chromium profiler and took some heap dumps, noticed tons of ResizeObserver references that were not eligible for GC. Eventually traced it back to this.

If the resulting ref was null in _refSetter(), the old reference wasn't getting reset, and we'd end up re-observing the original, stale node. This seems to have been preventing garbage collection.

After making this change, memory usage seems stable.

If the resulting ref was null, the old reference wasn't getting reset, and we'd end up re-observing the original node.

@bvaughn bvaughn left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey thanks @clangen-nw!

Comment thread src/ItemMeasurer.js
this._didProvideValidRef = true;
this._node = ref;
} else if (ref !== null) {
} else if (ref) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change? Seems like ref !== null should still be valid?

@bvaughn bvaughn merged commit f866022 into bvaughn:issues/6 May 27, 2019
@bvaughn

bvaughn commented May 27, 2019

Copy link
Copy Markdown
Owner

Cool, I dig it. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants