Conversation
src/util/hb_arena.c
Outdated
| #include <string.h> | ||
|
|
||
| #define hb_arena_for_each_page(allocator, page) \ | ||
| #define hb_arena_for_each_page(allocator, _page) \ |
There was a problem hiding this comment.
Would it make sense to remove the unused argument or do you have future plans for it? The macro name doesn't suggest that.
There was a problem hiding this comment.
The point was to self-document the variable that's going to be available inside the loop. But maybe that's confusing?
hb_arena_for_each_page(arena, page) {
total += page->position;
}vs:
hb_arena_for_each_page(arena) {
total += page->position;
// ^ where is page coming from?
}There was a problem hiding this comment.
I'd say it is slightly confusing because clangd sees it as unused. So to me it looks less self-documenting and more like a leftover from an older implementation. But there are bigger fish to fry. 😃
commit: |
🌿 Interactive Playground and Documentation PreviewA preview deployment has been built for this pull request. Try out the changes live in the interactive playground: 🌱 Grown from commit ✅ Preview deployment has been cleaned up. |
This pull request extracts a `hb_allocator_T` for providing an interface for allocating memory in the lexer and parser. In this pull request we implement both a `malloc`-based and a `hb_arena_T`-based allocator. Additionally, this updates all call-sites and functions to accept a new allocator that can later be swapped to use the `hb_arena_T`-based allocator in #726.
Context: https: //github.com//pull/726#pullrequestreview-3811782126 Co-Authored-By: Michael Kohl <me@citizen428.net>
Context: https: //github.com//pull/726#pullrequestreview-3811782126 Co-Authored-By: Michael Kohl <me@citizen428.net>
This pull request introduces arena allocation for the lexer and parser, replacing "per-object"
malloc/freecalls with bulk allocation from a memory arena.All allocated AST nodes, tokens, and internal strings are placed into a single arena that is freed in one shot after the parse tree has been converted to the binding's native objects.
The arena is accessed through
hb_allocator_T, a vtable-based allocator abstraction introduced in #1287. This pull request switches the default backend frommalloctohb_arena_Tacross all bindings and the CLI.The
mallocbackend remains available as a fallback. The intent is to validate the arena approach in production and then, once confident, simplify the abstraction away and usehb_arena_Tdirectly.The
hb_allocator_Tinterface was extended with adestroyfunction pointer and a high-levelhb_allocator_init(allocator, type)constructor that takes anhb_allocator_type_Tenum (HB_ALLOCATOR_ARENA,HB_ALLOCATOR_MALLOC).A separate
hb_allocator_init_with_sizevariant accepts a custom initial arena size for edge cases. The default arena size is controlled by a compile-timeHB_ALLOCATOR_DEFAULT_ARENA_SIZEflag.The extract API (
herb_extract,herb_extract_ruby_*,herb_extract_html_*) was also updated to accept anhb_allocator_T*.Depends on #1287