From 666a5c07b8d527578619dc4041aa63c11f4b6060 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 5 Nov 2026 08:35:49 -0500 Subject: [PATCH 1/2] Remove capacity from constant id lists --- CHANGELOG.md | 1 + include/prism/constant_pool.h | 20 +++++++---- include/prism/internal/constant_pool.h | 7 ++-- rust/ruby-prism-sys/src/bindings.rs | 9 +++-- rust/ruby-prism/src/node.rs | 4 +-- src/constant_pool.c | 50 +++++++++----------------- src/prism.c | 6 ++-- 7 files changed, 42 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7148d874f8..b27223fccd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a - `Prism::Translation::Parser` now returns a `nil` AST whenever an error is reported and errors are not fatal, matching the parser gem. - `PRISM_BUILD_MINIMAL` now excludes the full set of encodings, as `PRISM_ENCODING_EXCLUDE_FULL` does. - `pm_node_list_t` no longer has a `capacity` field, and its `size` field is now a `uint32_t`. +- `pm_constant_id_list_t` no longer has a `capacity` field, and its `size` field is now a `uint32_t`. ## [2.1.0] - 2026-10-28 diff --git a/include/prism/constant_pool.h b/include/prism/constant_pool.h index 61221ca9b3..9b09d22970 100644 --- a/include/prism/constant_pool.h +++ b/include/prism/constant_pool.h @@ -49,11 +49,12 @@ pm_constant_id_index(pm_constant_id_t constant_id) { * A list of constant IDs. Usually used to represent a set of locals. */ typedef struct { - /** The number of constant ids in the list. */ - size_t size; - - /** The number of constant ids that have been allocated in the list. */ - size_t capacity; + /** + * The number of constant ids in the list. Source offsets are 32 bits, so + * the source is less than 4 GiB, and a list built from it stays far below + * what this can count. + */ + uint32_t size; /** The constant ids in the list. */ pm_constant_id_t *ids; @@ -86,12 +87,17 @@ PRISM_EXPORTED_FUNCTION size_t pm_constant_length(const pm_constant_t *constant) /** * Initialize a list of constant ids. * + * @param arena The arena to use for allocations. * @param list The list to initialize. + * @param size The size of the list. */ -PRISM_EXPORTED_FUNCTION void pm_constant_id_list_init(pm_constant_id_list_t *list) PRISM_NONNULL(1); +PRISM_EXPORTED_FUNCTION void pm_constant_id_list_init(pm_arena_t *arena, pm_constant_id_list_t *list, uint32_t size) PRISM_NONNULL(1, 2); /** - * Append a constant id to a list of constant ids. + * Append a constant id to a list of constant ids. A list's storage is always + * exactly its size, so it is regrown to exactly one more id by allocating from + * the given arena, which must live as long as the list does. For a list in a + * parsed tree, that is the arena that was passed to pm_parser_new. * * @param arena The arena to use for allocations. * @param list The list to append to. diff --git a/include/prism/internal/constant_pool.h b/include/prism/internal/constant_pool.h index 459da08056..f6d227f3b7 100644 --- a/include/prism/internal/constant_pool.h +++ b/include/prism/internal/constant_pool.h @@ -75,11 +75,8 @@ struct pm_constant_pool_t { */ #define PM_CONSTANT_ID_UNSET 0 -/* Initialize a list of constant ids with a given capacity. */ -void pm_constant_id_list_init_capacity(pm_arena_t *arena, pm_constant_id_list_t *list, size_t capacity); - -/* Insert a constant id into a list of constant ids at the specified index. */ -void pm_constant_id_list_insert(pm_constant_id_list_t *list, size_t index, pm_constant_id_t id); +/* Set the unset constant id at the specified index of a list of constant ids. */ +void pm_constant_id_list_insert(pm_constant_id_list_t *list, uint32_t index, pm_constant_id_t id); /* * Mix the bits of a constant id so that ids which are close together, as diff --git a/rust/ruby-prism-sys/src/bindings.rs b/rust/ruby-prism-sys/src/bindings.rs index 889aaf3646..09ca6562d4 100644 --- a/rust/ruby-prism-sys/src/bindings.rs +++ b/rust/ruby-prism-sys/src/bindings.rs @@ -2598,12 +2598,11 @@ pub struct pm_arena_t { #[repr(C)] #[derive(Debug, Copy, Clone)] pub struct pm_constant_id_list_t { - /** The number of constant ids in the list. + /** The number of constant ids in the list. Source offsets are 32 bits, so + the source is less than 4 GiB, and a list built from it stays far below + what this can count. */ - pub size: usize, - /** The number of constant ids that have been allocated in the list. -*/ - pub capacity: usize, + pub size: u32, /** The constant ids in the list. */ pub ids: *mut pm_constant_id_t, diff --git a/rust/ruby-prism/src/node.rs b/rust/ruby-prism/src/node.rs index ea5ed841b5..9a205228cc 100644 --- a/rust/ruby-prism/src/node.rs +++ b/rust/ruby-prism/src/node.rs @@ -159,7 +159,7 @@ impl<'pr> Iterator for ConstantListIter<'pr> { type Item = ConstantId<'pr>; fn next(&mut self) -> Option { - if self.index >= unsafe { self.pointer.as_ref().size } { + if self.index >= unsafe { self.pointer.as_ref().size } as usize { None } else { let constant_id: pm_constant_id_t = unsafe { *(self.pointer.as_ref().ids.add(self.index)) }; @@ -201,7 +201,7 @@ impl<'pr> ConstantList<'pr> { /// Returns the length of the list. #[must_use] pub const fn len(&self) -> usize { - unsafe { self.pointer.as_ref().size } + unsafe { self.pointer.as_ref().size as usize } } /// Returns whether the list is empty. diff --git a/src/constant_pool.c b/src/constant_pool.c index c1e723063a..25541d9737 100644 --- a/src/constant_pool.c +++ b/src/constant_pool.c @@ -11,57 +11,41 @@ * Initialize a list of constant ids. */ void -pm_constant_id_list_init(pm_constant_id_list_t *list) { - list->ids = NULL; - list->size = 0; - list->capacity = 0; -} - -/** - * Initialize a list of constant ids with a given capacity. - */ -void -pm_constant_id_list_init_capacity(pm_arena_t *arena, pm_constant_id_list_t *list, size_t capacity) { - if (capacity) { - list->ids = (pm_constant_id_t *) pm_arena_zalloc(arena, capacity * sizeof(pm_constant_id_t), PRISM_ALIGNOF(pm_constant_id_t)); +pm_constant_id_list_init(pm_arena_t *arena, pm_constant_id_list_t *list, uint32_t size) { + if (size) { + list->ids = (pm_constant_id_t *) pm_arena_zalloc(arena, size * sizeof(pm_constant_id_t), PRISM_ALIGNOF(pm_constant_id_t)); } else { list->ids = NULL; } - list->size = 0; - list->capacity = capacity; + list->size = size; } /** - * Append a constant id to a list of constant ids. + * Append a constant id to a list of constant ids. A list's array is always + * exactly its size, so it grows to exactly one more id, in place when it is the + * most recent allocation in the arena and by copying it otherwise. */ void pm_constant_id_list_append(pm_arena_t *arena, pm_constant_id_list_t *list, pm_constant_id_t id) { - if (list->size >= list->capacity) { - size_t new_capacity = list->capacity == 0 ? 8 : list->capacity * 2; - pm_constant_id_t *new_ids = (pm_constant_id_t *) pm_arena_alloc(arena, sizeof(pm_constant_id_t) * new_capacity, PRISM_ALIGNOF(pm_constant_id_t)); - - if (list->size > 0) { - memcpy(new_ids, list->ids, list->size * sizeof(pm_constant_id_t)); - } - - list->ids = new_ids; - list->capacity = new_capacity; + if (list->size == 0) { + list->ids = (pm_constant_id_t *) pm_arena_alloc(arena, sizeof(pm_constant_id_t), PRISM_ALIGNOF(pm_constant_id_t)); + list->ids[list->size++] = id; + } else { + list->ids =(pm_constant_id_t *) pm_arena_realloc(arena, list->ids, sizeof(pm_constant_id_t) * (list->size + 1), sizeof(pm_constant_id_t) * list->size, PRISM_ALIGNOF(pm_constant_id_t)); + list->ids[list->size++] = id; } - - list->ids[list->size++] = id; } /** - * Insert a constant id into a list of constant ids at the specified index. + * Set the constant id at the specified index of a list of constant ids. The + * slot must still be unset. */ void -pm_constant_id_list_insert(pm_constant_id_list_t *list, size_t index, pm_constant_id_t id) { - assert(index < list->capacity); +pm_constant_id_list_insert(pm_constant_id_list_t *list, uint32_t index, pm_constant_id_t id) { + assert(index < list->size); assert(list->ids[index] == PM_CONSTANT_ID_UNSET); - list->ids[index] = id; - list->size++; } /** diff --git a/src/prism.c b/src/prism.c index 2fb51fbcbc..2beecc5532 100644 --- a/src/prism.c +++ b/src/prism.c @@ -1073,7 +1073,7 @@ pm_locals_reads(pm_locals_t *locals, pm_constant_id_t name) { */ static void pm_locals_order(pm_parser_t *parser, pm_locals_t *locals, pm_constant_id_list_t *list, bool toplevel) { - pm_constant_id_list_init_capacity(parser->arena, list, locals->size); + pm_constant_id_list_init(parser->arena, list, locals->size); // If we're still below the threshold for switching to a hash, then we only // need to loop over the locals until we hit the size because the locals are @@ -1088,7 +1088,7 @@ pm_locals_order(pm_parser_t *parser, pm_locals_t *locals, pm_constant_id_list_t pm_local_t *local = &locals->locals[index]; if (local->name != PM_CONSTANT_ID_UNSET) { - pm_constant_id_list_insert(list, (size_t) local->index, local->name); + pm_constant_id_list_insert(list, local->index, local->name); if (warn_unused && local->reads == 0 && ((parser->start_line >= 0) || (pm_line_offset_list_line(&parser->line_offsets, local->location.start, parser->start_line) >= 0))) { pm_constant_t *constant = pm_constant_pool_id_to_constant(&parser->constant_pool, local->name); @@ -5483,7 +5483,7 @@ pm_match_write_node_create(pm_parser_t *parser, pm_call_node_t *call, pm_node_li */ static pm_module_node_t * pm_module_node_create(pm_parser_t *parser, pm_constant_id_list_t *locals, const pm_token_t *module_keyword, pm_node_t *constant_path, const pm_token_t *name, pm_node_t *body, const pm_token_t *end_keyword) { - pm_constant_id_list_t module_locals = { .ids = NULL, .size = 0, .capacity = 0 }; + pm_constant_id_list_t module_locals = { .ids = NULL, .size = 0 }; if (locals != NULL) module_locals = *locals; return pm_module_node_new( From 08c411b24d3a19f210ad1c3dd14a27138cc7f61f Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 5 Nov 2026 12:50:22 -0500 Subject: [PATCH 2/2] Size the constant pool by the square root of the source --- include/prism/internal/constant_pool.h | 28 +++++---- src/constant_pool.c | 63 ++++++++++++++------ src/prism.c | 79 ++++++++++++++++++++------ 3 files changed, 124 insertions(+), 46 deletions(-) diff --git a/include/prism/internal/constant_pool.h b/include/prism/internal/constant_pool.h index f6d227f3b7..39bb63a58f 100644 --- a/include/prism/internal/constant_pool.h +++ b/include/prism/internal/constant_pool.h @@ -32,7 +32,12 @@ static const pm_constant_pool_bucket_type_t PM_CONSTANT_POOL_BUCKET_OWNED = 1; /* A constant constant is known at compile time. */ static const pm_constant_pool_bucket_type_t PM_CONSTANT_POOL_BUCKET_CONSTANT = 2; -/* A bucket in the hash map. */ +/* + * A bucket in the hash map. It holds no string, so that it stays eight bytes + * wide for every slot of a mostly empty table. A probe compares hashes first, + * and reads the string through the constants array only when they match, which + * is almost always a real hit. + */ typedef struct { /* The incremental ID used for indexing back into the pool. */ unsigned int id: 30; @@ -42,15 +47,6 @@ typedef struct { /* The hash of the bucket. */ uint32_t hash; - - /* - * A pointer to the start of the string, stored directly in the bucket to - * avoid a pointer chase to the constants array during probing. - */ - const uint8_t *start; - - /* The length of the string. */ - size_t length; } pm_constant_pool_bucket_t; /* The overall constant pool, which stores constants found while parsing. */ @@ -58,7 +54,11 @@ struct pm_constant_pool_t { /* The buckets in the hash map. */ pm_constant_pool_bucket_t *buckets; - /* The constants that are stored in the buckets. */ + /* + * The constants that are stored in the buckets, indexed by id minus one. + * The pool grows before it is three quarters full, so this has room for + * three quarters as many constants as there are buckets, rounded up. + */ pm_constant_t *constants; /* The number of buckets in the hash map. */ @@ -118,6 +118,12 @@ bool pm_constant_id_set_insert(pm_arena_t *arena, pm_constant_id_set_t *set, pm_ /* Initialize a new constant pool with a given capacity. */ void pm_constant_pool_init(pm_arena_t *arena, pm_constant_pool_t *pool, uint32_t capacity); +/* + * Return the number of bytes that pm_constant_pool_init allocates for the given + * capacity. + */ +size_t pm_constant_pool_init_size(uint32_t capacity); + /* Return a pointer to the constant indicated by the given constant id. */ pm_constant_t * pm_constant_pool_id_to_constant(const pm_constant_pool_t *pool, pm_constant_id_t constant_id); diff --git a/src/constant_pool.c b/src/constant_pool.c index 25541d9737..2b56792f7b 100644 --- a/src/constant_pool.c +++ b/src/constant_pool.c @@ -192,6 +192,29 @@ is_power_of_two(uint32_t size) { } #endif +/** + * Return the number of constants that a pool with the given capacity has room + * for. The pool grows before an insert once it is three quarters full, so at + * most three quarters of its capacity, rounded up, ever holds a constant. + */ +static PRISM_INLINE size_t +pm_constant_pool_constants_capacity(uint32_t capacity) { + return (size_t) (capacity - capacity / 4); +} + +/** + * Return whether the given bucket holds the given string. The stored hash is + * compared first, so that probe collisions are rejected without reading the + * constant or calling memcmp. + */ +static PRISM_INLINE bool +pm_constant_pool_bucket_match(const pm_constant_pool_t *pool, const pm_constant_pool_bucket_t *bucket, uint32_t hash, const uint8_t *start, size_t length) { + if (bucket->hash != hash) return false; + + const pm_constant_t *constant = &pool->constants[bucket->id - 1]; + return constant->length == length && memcmp(constant->start, start, length) == 0; +} + /** * Resize a constant pool to a given capacity. */ @@ -202,8 +225,15 @@ pm_constant_pool_resize(pm_arena_t *arena, pm_constant_pool_t *pool) { uint32_t next_capacity = pool->capacity * 2; const uint32_t mask = next_capacity - 1; + /* The constants are indexed by id, which a resize does not change, so + * they only need more room. They grow before the buckets are allocated, + * which lets them grow in place when they are the most recent allocation + * in the arena. */ + size_t constants_size = pm_constant_pool_constants_capacity(pool->capacity) * sizeof(pm_constant_t); + size_t next_constants_size = pm_constant_pool_constants_capacity(next_capacity) * sizeof(pm_constant_t); + pool->constants = (pm_constant_t *) pm_arena_realloc(arena, pool->constants, next_constants_size, constants_size, PRISM_ALIGNOF(pm_constant_t)); + pm_constant_pool_bucket_t *next_buckets = (pm_constant_pool_bucket_t *) pm_arena_zalloc(arena, next_capacity * sizeof(pm_constant_pool_bucket_t), PRISM_ALIGNOF(pm_constant_pool_bucket_t)); - pm_constant_t *next_constants = (pm_constant_t *) pm_arena_alloc(arena, next_capacity * sizeof(pm_constant_t), PRISM_ALIGNOF(pm_constant_t)); // For each bucket in the current constant pool, find the index in the // next constant pool, and insert it. @@ -228,10 +258,6 @@ pm_constant_pool_resize(pm_arena_t *arena, pm_constant_pool_t *pool) { } } - // The constants are stable with respect to hash table resizes. - memcpy(next_constants, pool->constants, pool->size * sizeof(pm_constant_t)); - - pool->constants = next_constants; pool->buckets = next_buckets; pool->capacity = next_capacity; } @@ -242,13 +268,22 @@ pm_constant_pool_resize(pm_arena_t *arena, pm_constant_pool_t *pool) { void pm_constant_pool_init(pm_arena_t *arena, pm_constant_pool_t *pool, uint32_t capacity) { capacity = next_power_of_two(capacity); - pool->buckets = (pm_constant_pool_bucket_t *) pm_arena_zalloc(arena, capacity * sizeof(pm_constant_pool_bucket_t), PRISM_ALIGNOF(pm_constant_pool_bucket_t)); - pool->constants = (pm_constant_t *) pm_arena_alloc(arena, capacity * sizeof(pm_constant_t), PRISM_ALIGNOF(pm_constant_t)); + pool->constants = (pm_constant_t *) pm_arena_alloc(arena, pm_constant_pool_constants_capacity(capacity) * sizeof(pm_constant_t), PRISM_ALIGNOF(pm_constant_t)); pool->size = 0; pool->capacity = capacity; } +/** + * Return the number of bytes that pm_constant_pool_init allocates for the given + * capacity. + */ +size_t +pm_constant_pool_init_size(uint32_t capacity) { + capacity = next_power_of_two(capacity); + return capacity * sizeof(pm_constant_pool_bucket_t) + pm_constant_pool_constants_capacity(capacity) * sizeof(pm_constant_t); +} + /** * Return a pointer to the constant indicated by the given constant id. */ @@ -272,9 +307,7 @@ pm_constant_pool_find(const pm_constant_pool_t *pool, const uint8_t *start, size pm_constant_pool_bucket_t *bucket; while (bucket = &pool->buckets[index], bucket->id != PM_CONSTANT_ID_UNSET) { - // Compare the stored hash before touching the contents so that probe - // collisions are rejected without a memcmp call. - if ((bucket->hash == hash) && (bucket->length == length) && memcmp(bucket->start, start, length) == 0) { + if (pm_constant_pool_bucket_match(pool, bucket, hash, start, length)) { return bucket->id; } @@ -303,9 +336,8 @@ pm_constant_pool_insert(pm_arena_t *arena, pm_constant_pool_t *pool, const uint8 while (bucket = &pool->buckets[index], bucket->id != PM_CONSTANT_ID_UNSET) { // If there is a collision, then we need to check if the content is the // same as the content we are trying to insert. If it is, then we can - // return the id of the existing constant. Compare the stored hash - // first so that probe collisions are rejected without a memcmp call. - if ((bucket->hash == hash) && (bucket->length == length) && memcmp(bucket->start, start, length) == 0) { + // return the id of the existing constant. + if (pm_constant_pool_bucket_match(pool, bucket, hash, start, length)) { // Since we have found a match, we need to check if this is // attempting to insert a shared or an owned constant. We want to // prefer shared constants since they don't require allocations. @@ -313,7 +345,6 @@ pm_constant_pool_insert(pm_arena_t *arena, pm_constant_pool_t *pool, const uint8 // If we're attempting to insert a shared constant and the // existing constant is owned, then we can replace it with the // shared constant to prefer non-owned references. - bucket->start = start; bucket->type = (unsigned int) (type & 0x3); pool->constants[bucket->id - 1].start = start; } @@ -332,9 +363,7 @@ pm_constant_pool_insert(pm_arena_t *arena, pm_constant_pool_t *pool, const uint8 *bucket = (pm_constant_pool_bucket_t) { .id = (unsigned int) (id & 0x3fffffff), .type = (unsigned int) (type & 0x3), - .hash = hash, - .start = start, - .length = length + .hash = hash }; pool->constants[id - 1] = (pm_constant_t) { diff --git a/src/prism.c b/src/prism.c index 2beecc5532..0898430f70 100644 --- a/src/prism.c +++ b/src/prism.c @@ -22924,6 +22924,32 @@ pm_parser_init_shebang(pm_parser_t *parser, const pm_options_t *options, const c #define PM_CONSTANT_POOL_MIN_CAPACITY 4 #endif +/** + * Return the integer square root of the given value, rounded down. This finds + * the result one bit at a time from the highest, so it takes at most half as + * many steps as there are bits in a size_t. + */ +static size_t +pm_size_sqrt(size_t value) { + size_t result = 0; + size_t bit = ((size_t) 1) << (sizeof(size_t) * 8 - 2); + + while (bit > value) bit >>= 2; + + while (bit != 0) { + if (value >= result + bit) { + value -= result + bit; + result = (result >> 1) + bit; + } else { + result >>= 1; + } + + bit >>= 2; + } + + return result; +} + /** * Initialize a parser with the given start and end pointers. */ @@ -22990,28 +23016,45 @@ pm_parser_init(pm_arena_t *arena, pm_parser_t *parser, const uint8_t *source, si .warn_mismatched_indentation = true }; - /* Pre-size the arenas based on input size to reduce the number of block - * allocations (and the kernel page zeroing they trigger). The ratios were - * measured empirically: AST arena ~3.3x input, metadata arena ~1.1x input. - * The reserve call is a no-op when the capacity is at or below the default - * arena block size, so small inputs don't waste an extra allocation. */ + /* Pre-size the AST arena based on input size to reduce the number of block + * allocations (and the kernel page zeroing they trigger). The tree grows + * linearly with the source. Measured across 16,460 stdlib and gem files, + * it takes about 4.6 bytes per source byte at the median, from about 3.3 + * at the 25th percentile to about 6 at the 75th, depending on how much of + * the source is comments and how much is literals. A source past the + * reserve continues in further blocks. The reserve call is a no-op when + * the capacity is at or below the default arena block size, so small + * inputs don't waste an extra allocation. */ if (size <= SIZE_MAX / 4) pm_arena_reserve(arena, size * 4); - if (size <= SIZE_MAX / 5 * 4) pm_arena_reserve(&parser->metadata_arena, size + size / 4); - - /* Initialize the constant pool. Measured across 1532 Ruby stdlib files, the - * bytes/constant ratio has a median of ~56 and a 90th percentile of ~135. - * We use 120 as a balance between over-allocation waste and resize - * frequency. Resizes are cheap with arena allocation, so we lean toward - * under-estimating. */ - uint32_t constant_size = ((uint32_t) size) / 120; + + /* The number of distinct names in a source grows with the square root of + * its size, not with its size, because longer sources mostly reuse names. + * Measured across 16,460 stdlib and gem files, a source holds about + * sqrt(size) names at the median and 1.5 * sqrt(size) at the 90th + * percentile. Starting the pool at twice that keeps nearly every source + * below the load at which the pool grows, and each growth leaves the + * outgrown buckets behind in the metadata arena. Ids are 30 bits wide, + * which bounds the capacity. */ + size_t constant_estimate = pm_size_sqrt(size) * 2; + if (constant_estimate > ((size_t) 1 << 30)) constant_estimate = (size_t) 1 << 30; + + uint32_t constant_size = (uint32_t) constant_estimate; if (constant_size < PM_CONSTANT_POOL_MIN_CAPACITY) constant_size = PM_CONSTANT_POOL_MIN_CAPACITY; - pm_constant_pool_init(&parser->metadata_arena, &parser->constant_pool, constant_size); - /* Initialize the line offset list. Similar to the constant pool, we are - * going to estimate the number of newlines that we will need based on the - * size of the input. */ + /* Estimate the number of newlines from the size of the input. */ size_t newline_size = size / 22; - pm_line_offset_list_init(&parser->metadata_arena, &parser->line_offsets, newline_size < 4 ? 4 : newline_size); + if (newline_size < 4) newline_size = 4; + + /* Pre-size the metadata arena to hold the constant pool and the line + * offsets at the sizes they start with, so that they share its first + * block. Half of the input size on top covers what accumulates while + * parsing, which is mostly comments. */ + if (size <= SIZE_MAX / 2) { + pm_arena_reserve(&parser->metadata_arena, pm_constant_pool_init_size(constant_size) + newline_size * sizeof(uint32_t) + size / 2); + } + + pm_constant_pool_init(&parser->metadata_arena, &parser->constant_pool, constant_size); + pm_line_offset_list_init(&parser->metadata_arena, &parser->line_offsets, newline_size); // If options were provided to this parse, establish them here. if (options != NULL) {