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

record and release old wgpu BindGroup items, which were otherwise not being released. #1435

Merged
merged 2 commits into from
Jan 17, 2025

Conversation

rcoreilly
Copy link
Member

@rcoreilly rcoreilly commented Jan 16, 2025

Thanks to @AnyCPU for identifying leak and potential fix in #1434, and @StinkyPeach for reporting the issue.

Relevant to #1430

note that with screen zoom that there is a remaining issue associated with loading fonts for each different DPI size, which is not fixed with this PR, and will not be addressed until we do the vello and go-text refactor.

… being released. thanks to @AnyCPU for identifying leak and potential fix, and @StinkyPeach for reporting the issue.
@rcoreilly
Copy link
Member Author

I have now verified using the xcode instruments leak detector (had to turn system integrity protection off to run on go binaries) that this PR does fix the resizing memory leaks.

However, the special libwgpu_native.a library that I built using a PR to access a more recent wgpu version, which is NOT released in our webgpu package, does still leak surface textures. This library runs compute jobs significantly faster, but is not anything official, so hopefully they will come out with updated wgpu_native packages sometime soon, that actually work.

Even with the "good" released version of the wgpu_native library, there is an initial bump in memory usage at the point of first resizing, but it stabilizes from that point onward.

Copy link
Member

@kkoreilly kkoreilly left a comment

Choose a reason for hiding this comment

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

LGTM overall, and the memory leak does seem to be gone for me when resizing, but there is still an unequivocal persistent memory leak when zooming the window in and out, which was the original report in #1430.

@kkoreilly
Copy link
Member

Note that the same zooming memory leak appears to occur in #1434 as well. The fix could be a separate PR, but this PR then would not be a fix for #1430.

@rcoreilly
Copy link
Member Author

it is the same situation as the resize:

image

an initial bump and then transients that re-stabilize. this plot from xcode leaks looks identical to resize.

@kkoreilly kkoreilly merged commit a62baf5 into main Jan 17, 2025
1 check passed
@kkoreilly kkoreilly deleted the gpubind branch January 17, 2025 18:56
@kkoreilly
Copy link
Member

See #1430 (comment) for information about the remaining leaking when zooming.

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