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

unexpected memory behaviour when relying on runtime.SetFinalizer vs explicit Close #181

Open
timruffles opened this issue Jan 10, 2025 · 0 comments

Comments

@timruffles
Copy link

timruffles commented Jan 10, 2025

Currently, unless Go GC is frequent, the finalizers may not run, and thus the sitter C types will not be freed. Critically, if the bulk of a program's allocations comes from tree sitter, the process can use huge amounts of memory but Go GC won't run as the Go heap isn't growing. Here's a test case that demonstrates this nuance in runtime.SetFinalizer: https://gist.github.com/timruffles/48bfa3bd5a625d80e7c73d0eacfc8335

Suggestion: update the documentation to recommend people Close() treesitter types where possible, to avoid unexpected memory consumption. The finalizer can remain as a backstop, the pattern recommended in this comment from a go maintainer. Other Go common APIs make use of the defer foo.Close() pattern - e.g. files, HTTP responses - so it doesn't appear to me be an undue burden for users. I believe this will remain the more reliable approach even after runtime.AddCleanup is introduced, deprecating runtime.SetFinalizer, as it will have same behaviour as in the test case above.

This isn't something that'll become immediately obvious in one-shot use-cases of treesitter, but becomes more problematic for long-lived use cases, e.g. editors, LSP servers.

In any case, food for thought, and thanks for go-tree-sitter! 🙌

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

No branches or pull requests

1 participant