Skip to content

Conversation

efritz
Copy link

@efritz efritz commented Aug 20, 2025

Currently, every allocation from tree-sitter calls back to a Go shim for alloc/calloc/realloc/free operations. If SetAllocator is called with all nil values (the default), this has the effect of calling C.{alloc,calloc,realloc,free} and keeps all memory operations on the C side. However, this also incurs a C -> Go -> C boundary crossing for every allocation and free, which can be massively expensive when parsing at scale.

This PR changes the behavior of SetAllocator to remove the C -> Go -> C trampoline when there are no Go-side shims to execute. If any of the allocation parameters are non-nil, we'll fall back to the current behavior of installing shims for all four methods. I'm assuming common use of this is to replace all of these methods uniformly.

On a representative parse over 30,940 files, I see a decrease from 22,615ms to 5,517ms with this simple change. And see the following disappear completely from the CPU profile:

     0.03s 0.028% 98.25%     15.77s 14.85%  _cgoexp_7843849ca2c7_go_free
     0.03s 0.028% 98.28%     20.09s 18.91%  _cgoexp_7843849ca2c7_go_malloc

@jbedard
Copy link

jbedard commented Aug 26, 2025

Would this resolve #32? Partially?

@arnaldur
Copy link

arnaldur commented Sep 3, 2025

Do you experience that the memory is not released again to the system? When I monitor the go process running on my testdata with btop I see that the memory allocation on the process accumulates and is not released until the go process exits. Your branch is much faster on the test data I have compared to the v0.25.0 version of go-tree-sitter, but still having this memory issue.

@efritz
Copy link
Author

efritz commented Sep 9, 2025

@jbedard I think it does resolve #32, I don't see any additional callbacks from treesitter -> Go in my profiles.

@arnaldur If you could supply more info/a reproduction for me to look at I'd be happy to take a deeper look. I haven't seen memory issues in our production system (yet), but these are also longer-lived servers.

@arnaldur
Copy link

arnaldur commented Sep 9, 2025

Opened an issue with the relevant info: #36

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.

3 participants