-
Notifications
You must be signed in to change notification settings - Fork 32
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
Region-based allocation for AST nodes #122
Conversation
leaks Signed-off-by: SIGMazer <[email protected]>
Signed-off-by: SIGMazer <[email protected]>
Signed-off-by: SIGMazer <[email protected]>
@araujo88 can you take a look |
@SIGMazer can you explain in more detail how this region-based allocation works? Or provide a reference? |
We pre-allocate a memory chunk (Arena) and use it to allocate all Reference |
Very interesting! I wasn't familiar with that approach. Should we be concerned with the disadvantages? I understand that, as for the current usages of brainrot right now (which are small programs) this shouldn't be a concern.
|
I don't think this affects our case because we allocate a single contiguous region for all AST allocations, meaning we won't encounter any sparse regions. In the future, if we need to delete or transform nodes before freeing the entire Arena, we can introduce optimizations such as merging underutilized regions. Alternatively, we could use a bump allocator, which treats the region as a stack to avoid fragmentation. However, in our current case, I believe our implementation will not face any issues. |
Description
As the AST grows more complex, managing it becomes increasingly difficult, and free_ast becomes larger. This PR introduces a single Region to allocate all AST nodes, allowing them to be freed at once, simplifying memory management and reducing overhead.
Related Issue
Type of Change
Checklist