From a5a2646bb05f03d560df68ae814edc702de40880 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Sat, 24 Oct 2026 15:24:59 -0400 Subject: [PATCH] Scratch arena There are some constructs that only last for the duration of a parse, and should no longer be held once the parse is through. For those we should use a scratch arena, freed at the end of parsing. --- include/prism/internal/comments.h | 3 +- include/prism/internal/parser.h | 41 ++++++++++------ src/comments.c | 17 +++---- src/prism.c | 78 ++++++++++++++----------------- 4 files changed, 71 insertions(+), 68 deletions(-) diff --git a/include/prism/internal/comments.h b/include/prism/internal/comments.h index d546f566fc..4d42e20d3e 100644 --- a/include/prism/internal/comments.h +++ b/include/prism/internal/comments.h @@ -46,7 +46,8 @@ typedef struct { /* * Sort the parser's comment list in place by start offset, then assign every * comment to the node that owns it. This is called after parsing when the - * attach_comments option is set. + * attach_comments option is set, before the parser's scratch arena is released, + * because the walk allocates from it. */ void pm_comments_attach(pm_parser_t *parser, const pm_node_t *root); diff --git a/include/prism/internal/parser.h b/include/prism/internal/parser.h index 6404667318..7e47127b5b 100644 --- a/include/prism/internal/parser.h +++ b/include/prism/internal/parser.h @@ -272,13 +272,6 @@ typedef struct pm_lex_mode { struct pm_lex_mode *prev; } pm_lex_mode_t; -/* - * We pre-allocate a certain number of lex states in order to avoid having to - * call malloc too many times while parsing. You really shouldn't need more than - * this because you only really nest deeply when doing string interpolation. - */ -#define PM_LEX_STACK_SIZE 4 - /* * While parsing, we keep track of a stack of contexts. This is helpful for * error recovery so that we can pop back to a previous context when we hit a @@ -596,9 +589,20 @@ struct pm_parser_t { /* The arena used for all AST-lifetime allocations. Caller-owned. */ pm_arena_t *arena; - /* The arena used for parser metadata (comments, diagnostics, etc.). */ + /* + * The arena used for parser metadata (comments, diagnostics, etc.), which + * lives as long as the parser. + */ pm_arena_t metadata_arena; + /* + * The arena used for state that is only needed while parsing, such as + * scopes, locals, and contexts. It is released when pm_parse returns, so + * nothing allocated in it can be reachable from the tree or from the + * parser's metadata. + */ + pm_arena_t scratch_arena; + /* * The next node identifier that will be assigned. This is a unique * identifier used to track nodes such that the syntax tree can be dropped @@ -648,16 +652,23 @@ struct pm_parser_t { */ pm_state_stack_t accepts_block_stack; - /* A stack of lex modes. */ + /* A stack of lex modes, linked through their prev pointers. */ struct { /* The current mode of the lexer. */ pm_lex_mode_t *current; - /* The stack of lexer modes. */ - pm_lex_mode_t stack[PM_LEX_STACK_SIZE]; + /* + * The mode at the bottom of the stack, which is never popped. Every + * other mode is allocated from the scratch arena. + */ + pm_lex_mode_t base; - /* The current index into the lexer mode stack. */ - size_t index; + /* + * Lex modes that have been popped, kept for reuse by the next push. + * They are allocated from the scratch arena, so they are never freed + * individually. + */ + pm_lex_mode_t *free; } lex_modes; /* The pointer to the start of the source. */ @@ -717,7 +728,7 @@ struct pm_parser_t { /* * Scopes that have been popped, kept for reuse by the next push. Scopes - * are allocated from the metadata arena, so they are never freed + * are allocated from the scratch arena, so they are never freed * individually. */ pm_scope_t *free_scopes; @@ -727,7 +738,7 @@ struct pm_parser_t { /* * Context nodes that have been popped, kept for reuse by the next push. - * Context nodes are allocated from the metadata arena, so they are never + * Context nodes are allocated from the scratch arena, so they are never * freed individually. */ pm_context_node_t *free_contexts; diff --git a/src/comments.c b/src/comments.c index f011ba346b..ebd99be497 100644 --- a/src/comments.c +++ b/src/comments.c @@ -11,7 +11,10 @@ /* The state used while walking the tree to find the owner of each comment. */ typedef struct { - /* The arena for memory that is only needed during the walk. */ + /* + * The arena for memory that is only needed during the walk, which is the + * scratch arena of the parser. + */ pm_arena_t *arena; /* The start of the source that was parsed. */ @@ -389,15 +392,15 @@ pm_comments_attach(pm_parser_t *parser, const pm_node_t *root) { uint32_t size = (uint32_t) parser->comment_list.size; if (size == 0) return; - pm_arena_t arena = { 0 }; + pm_arena_t *arena = &parser->scratch_arena; pm_comments_walk_t walk = { - .arena = &arena, + .arena = arena, .source = parser->start, .comments = pm_comments_sort(parser, size), .size = size, - .starts = (uint32_t *) pm_arena_alloc(&arena, sizeof(uint32_t) * size, PRISM_ALIGNOF(uint32_t)), - .attached = (const pm_node_t **) pm_arena_zalloc(&arena, sizeof(pm_node_t *) * size, PRISM_ALIGNOF(pm_node_t *)), - .unclaimed = (uint32_t *) pm_arena_alloc(&arena, sizeof(uint32_t) * (size + 1), PRISM_ALIGNOF(uint32_t)) + .starts = (uint32_t *) pm_arena_alloc(arena, sizeof(uint32_t) * size, PRISM_ALIGNOF(uint32_t)), + .attached = (const pm_node_t **) pm_arena_zalloc(arena, sizeof(pm_node_t *) * size, PRISM_ALIGNOF(pm_node_t *)), + .unclaimed = (uint32_t *) pm_arena_alloc(arena, sizeof(uint32_t) * (size + 1), PRISM_ALIGNOF(uint32_t)) }; for (uint32_t index = 0; index < size; index++) walk.starts[index] = walk.comments[index]->location.start; @@ -443,8 +446,6 @@ pm_comments_attach(pm_parser_t *parser, const pm_node_t *root) { attachments[range->index + range->length++] = index; } - pm_arena_cleanup(&arena); - parser->comment_attachments = (pm_comment_attachments_t) { .comments = walk.comments, .attachments = attachments, diff --git a/src/prism.c b/src/prism.c index 2f6a519abd..da944fa282 100644 --- a/src/prism.c +++ b/src/prism.c @@ -184,27 +184,22 @@ lex_mode_terminator(const uint8_t start) { } /** - * Push a new lex state onto the stack. If we're still within the pre-allocated - * space of the lex state stack, then we'll just use a new slot. Otherwise we'll - * allocate a new pointer and use that. + * Push a new lex state onto the stack, reusing a popped lex mode if there is + * one and allocating a new one otherwise. */ static void lex_mode_push(pm_parser_t *parser, pm_lex_mode_t lex_mode) { - lex_mode.prev = parser->lex_modes.current; - parser->lex_modes.index++; - - if (parser->lex_modes.index > PM_LEX_STACK_SIZE - 1) { - parser->lex_modes.current = (pm_lex_mode_t *) xmalloc(sizeof(pm_lex_mode_t)); - if (PRISM_UNLIKELY(parser->lex_modes.current == NULL)) { - fprintf(stderr, "prism: out of memory; aborting\n"); - abort(); - } + pm_lex_mode_t *next = parser->lex_modes.free; - *parser->lex_modes.current = lex_mode; + if (next != NULL) { + parser->lex_modes.free = next->prev; } else { - parser->lex_modes.stack[parser->lex_modes.index] = lex_mode; - parser->lex_modes.current = &parser->lex_modes.stack[parser->lex_modes.index]; + next = (pm_lex_mode_t *) pm_arena_alloc(&parser->scratch_arena, sizeof(pm_lex_mode_t), PRISM_ALIGNOF(pm_lex_mode_t)); } + + lex_mode.prev = parser->lex_modes.current; + *next = lex_mode; + parser->lex_modes.current = next; } /** @@ -357,22 +352,20 @@ lex_mode_push_string_eof(pm_parser_t *parser) { } /** - * Pop the current lex state off the stack. If we're within the pre-allocated - * space of the lex state stack, then we'll just decrement the index. Otherwise - * we'll free the current pointer and use the previous pointer. + * Pop the current lex state off the stack, keeping it for reuse by the next + * push. The base lex mode is never popped, so popping it instead resets it to + * the default mode. */ static void lex_mode_pop(pm_parser_t *parser) { - if (parser->lex_modes.index == 0) { - parser->lex_modes.current->mode = PM_LEX_DEFAULT; - } else if (parser->lex_modes.index < PM_LEX_STACK_SIZE) { - parser->lex_modes.index--; - parser->lex_modes.current = &parser->lex_modes.stack[parser->lex_modes.index]; + pm_lex_mode_t *popped = parser->lex_modes.current; + + if (popped == &parser->lex_modes.base) { + popped->mode = PM_LEX_DEFAULT; } else { - parser->lex_modes.index--; - pm_lex_mode_t *prev = parser->lex_modes.current->prev; - xfree_sized(parser->lex_modes.current, sizeof(pm_lex_mode_t)); - parser->lex_modes.current = prev; + parser->lex_modes.current = popped->prev; + popped->prev = parser->lex_modes.free; + parser->lex_modes.free = popped; } } @@ -709,7 +702,7 @@ pm_parser_scope_push(pm_parser_t *parser, bool closed) { memset(locals.locals, 0, locals.capacity * sizeof(pm_local_t)); } } else { - scope = (pm_scope_t *) pm_arena_alloc(&parser->metadata_arena, sizeof(pm_scope_t), PRISM_ALIGNOF(pm_scope_t)); + scope = (pm_scope_t *) pm_arena_alloc(&parser->scratch_arena, sizeof(pm_scope_t), PRISM_ALIGNOF(pm_scope_t)); } *scope = (pm_scope_t) { @@ -7262,7 +7255,7 @@ pm_parser_local_depth(pm_parser_t *parser, pm_token_t *token) { */ static PRISM_INLINE void pm_parser_local_add(pm_parser_t *parser, pm_constant_id_t constant_id, const uint8_t *start, const uint8_t *end, uint32_t reads) { - pm_locals_write(&parser->metadata_arena, &parser->current_scope->locals, constant_id, U32(start - parser->start), U32(end - start), reads); + pm_locals_write(&parser->scratch_arena, &parser->current_scope->locals, constant_id, U32(start - parser->start), U32(end - start), reads); } /** @@ -7997,7 +7990,7 @@ context_push(pm_parser_t *parser, pm_context_t context) { if (context_node != NULL) { parser->free_contexts = context_node->prev; } else { - context_node = (pm_context_node_t *) pm_arena_alloc(&parser->metadata_arena, sizeof(pm_context_node_t), PRISM_ALIGNOF(pm_context_node_t)); + context_node = (pm_context_node_t *) pm_arena_alloc(&parser->scratch_arena, sizeof(pm_context_node_t), PRISM_ALIGNOF(pm_context_node_t)); } pm_context_node_t *previous = parser->current_context; @@ -15981,7 +15974,7 @@ ensure_valid_jump(pm_parser_t *parser, pm_node_t *node) { // block exit to the list of exits for the expression, and // the node parsing will handle validating it instead. assert(parser->current_block_exits != NULL); - pm_node_list_append_inline(parser->arena, parser->current_block_exits, node); + pm_node_list_append_inline(&parser->scratch_arena, parser->current_block_exits, node); return; case PM_CONTEXT_BEGIN_ELSE: case PM_CONTEXT_BEGIN_ENSURE: @@ -16073,7 +16066,7 @@ pop_block_exits(pm_parser_t *parser, pm_node_list_t *previous_block_exits) { // However, they could still become valid in a higher level context if // there is another list above this one. In this case we'll push all of // the block exits up to the previous list. - pm_node_list_concat(parser->arena, previous_block_exits, parser->current_block_exits); + pm_node_list_concat(&parser->scratch_arena, previous_block_exits, parser->current_block_exits); parser->current_block_exits = previous_block_exits; } else { // If we did not match a trailing while/until and this was the last @@ -16719,12 +16712,12 @@ parse_variable(pm_parser_t *parser) { } pm_node_t *node = UP(pm_local_variable_read_node_create_constant_id(parser, &parser->previous, name_id, 0, false)); - pm_node_list_append_inline(parser->arena, ¤t_scope->implicit_parameters, node); + pm_node_list_append_inline(&parser->scratch_arena, ¤t_scope->implicit_parameters, node); return node; } else if ((parser->version >= PM_OPTIONS_VERSION_CRUBY_3_4) && pm_token_is_it(parser->previous.start, parser->previous.end)) { pm_node_t *node = UP(pm_it_local_variable_read_node_create(parser, &parser->previous)); - pm_node_list_append_inline(parser->arena, ¤t_scope->implicit_parameters, node); + pm_node_list_append_inline(&parser->scratch_arena, ¤t_scope->implicit_parameters, node); return node; } @@ -17111,7 +17104,7 @@ parse_pattern_capture(pm_parser_t *parser, pm_constant_id_set_t *captures, pm_co // Skip this capture if it starts with an underscore. if (peek_at(parser, parser->start + location->start) == '_') return; - if (!pm_constant_id_set_insert(parser->arena, captures, capture)) { + if (!pm_constant_id_set_insert(&parser->scratch_arena, captures, capture)) { pm_parser_err(parser, location->start, location->length, PM_ERR_PATTERN_CAPTURE_DUPLICATE); } } @@ -21804,7 +21797,7 @@ parse_regular_expression_named_capture(pm_parser_t *parser, const pm_string_t *c * Add this name to the set of constants if it is valid, not duplicated, * and not a keyword. */ - if (name != 0 && pm_constant_id_set_insert(parser->arena, names, name)) { + if (name != 0 && pm_constant_id_set_insert(&parser->scratch_arena, names, name)) { int depth; if ((depth = pm_parser_local_depth_constant_id(parser, name)) == -1) { @@ -22000,7 +21993,7 @@ parse_match(pm_parser_t *parser, pm_node_t *node, pm_binding_power_t binding_pow } if (!interpolated && total_length > 0) { - uint8_t *memory = (uint8_t *) pm_arena_alloc(&parser->metadata_arena, total_length, PRISM_ALIGNOF(uint8_t)); + uint8_t *memory = (uint8_t *) pm_arena_alloc(&parser->scratch_arena, total_length, PRISM_ALIGNOF(uint8_t)); uint8_t *cursor = memory; PM_NODE_LIST_FOREACH(parts, index, part) { pm_string_t *unescaped = &((pm_string_node_t *) part)->unescaped; @@ -23280,9 +23273,8 @@ pm_parser_init(pm_arena_t *arena, pm_parser_t *parser, const uint8_t *source, si .do_loop_stack = 0, .accepts_block_stack = 0, .lex_modes = { - .index = 0, - .stack = {{ .mode = PM_LEX_DEFAULT }}, - .current = &parser->lex_modes.stack[0], + .current = &parser->lex_modes.base, + .base = { .mode = PM_LEX_DEFAULT }, }, .start = source, .end = source + size, @@ -23549,10 +23541,7 @@ pm_parser_new(pm_arena_t *arena, const uint8_t *source, size_t size, const pm_op void pm_parser_cleanup(pm_parser_t *parser) { pm_arena_cleanup(&parser->metadata_arena); - - while (parser->lex_modes.index >= PM_LEX_STACK_SIZE) { - lex_mode_pop(parser); - } + pm_arena_cleanup(&parser->scratch_arena); } /** @@ -23734,6 +23723,7 @@ pm_parse(pm_parser_t *parser) { pm_node_t *node = parse_program(parser); pm_parse_continuable(parser); if (parser->attach_comments) pm_comments_attach(parser, node); + pm_arena_cleanup(&parser->scratch_arena); return node; }