From 99dc513d155484a1291394e953a5cdfd6dab92e1 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Sun, 17 May 2026 14:40:14 +0200 Subject: [PATCH] Use IMEMO to store `cdhash` RHash isn't a good fit for storing `cdhash` as this force to allow arbitrary hash types into RHash, which doesn't work with AR tables. It also cause the cdhash to be larger than needed. --- compile.c | 104 ++++++++++++++++++++++++--------- debug_counter.h | 1 + ext/objspace/objspace.c | 1 + gc.c | 1 + hash.c | 8 --- imemo.c | 31 ++++++++++ internal/hash.h | 1 - internal/imemo.h | 15 +++++ prism_compile.c | 7 +-- vm_insnhelper.c | 2 +- yjit.c | 24 ++++++++ yjit/bindgen/src/main.rs | 3 +- yjit/src/codegen.rs | 20 +------ yjit/src/cruby_bindings.inc.rs | 19 +++--- zjit/src/cruby_bindings.inc.rs | 1 + 15 files changed, 164 insertions(+), 74 deletions(-) diff --git a/compile.c b/compile.c index 1d5381b5feb..1ed3bffed12 100644 --- a/compile.c +++ b/compile.c @@ -2330,6 +2330,35 @@ static const struct st_hash_type cdhash_type = { rb_iseq_cdhash_hash, }; +static VALUE +cdhash_new(size_t size) +{ + VALUE cdhash = rb_imemo_cdhash_new(size, &cdhash_type); + RB_OBJ_SET_SHAREABLE(cdhash); + return cdhash; +} + +static void +cdhash_aset(VALUE cdhash, VALUE key, VALUE val) +{ + st_table *tbl = rb_imemo_cdhash_tbl(cdhash); + st_insert(tbl, key, val); + RB_OBJ_WRITTEN(cdhash, Qundef, key); + RB_OBJ_WRITTEN(cdhash, Qundef, val); +} + +static void +cdhash_aset_if_missing(VALUE cdhash, VALUE key, VALUE val) +{ + st_table *tbl = rb_imemo_cdhash_tbl(cdhash); + VALUE dontcare; + if (!st_lookup(tbl, key, &dontcare)) { + st_insert(tbl, key, val); + RB_OBJ_WRITTEN(cdhash, Qundef, key); + RB_OBJ_WRITTEN(cdhash, Qundef, val); + } +} + struct cdhash_set_label_struct { VALUE hash; int pos; @@ -2341,11 +2370,10 @@ cdhash_set_label_i(VALUE key, VALUE val, VALUE ptr) { struct cdhash_set_label_struct *data = (struct cdhash_set_label_struct *)ptr; LABEL *lobj = (LABEL *)(val & ~1); - rb_hash_aset(data->hash, key, INT2FIX(lobj->position - (data->pos+data->len))); + cdhash_aset(data->hash, key, INT2FIX(lobj->position - (data->pos+data->len))); return ST_CONTINUE; } - static inline VALUE get_ivar_ic_value(rb_iseq_t *iseq,ID id) { @@ -2729,10 +2757,8 @@ iseq_set_sequence(rb_iseq_t *iseq, LINK_ANCHOR *const anchor) data.hash = map; data.pos = code_index; data.len = len; - rb_hash_foreach(map, cdhash_set_label_i, (VALUE)&data); + st_foreach(rb_imemo_cdhash_tbl(map), cdhash_set_label_i, (VALUE)&data); - freeze_hide_obj(map); - rb_ractor_make_shareable(map); generated_iseq[code_index + 1 + j] = map; ISEQ_MBITS_SET(mark_offset_bits, code_index + 1 + j); RB_OBJ_WRITTEN(iseq, Qundef, map); @@ -5526,8 +5552,8 @@ when_vals(rb_iseq_t *iseq, LINK_ANCHOR *const cond_seq, const NODE *vals, if (UNDEF_P(lit)) { only_special_literals = 0; } - else if (NIL_P(rb_hash_lookup(literals, lit))) { - rb_hash_aset(literals, lit, (VALUE)(l1) | 1); + else { + cdhash_aset_if_missing(literals, lit, (VALUE)(l1) | 1); } if (nd_type_p(val, NODE_STR) || nd_type_p(val, NODE_FILE)) { @@ -7069,7 +7095,7 @@ compile_case(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const orig_nod DECL_ANCHOR(body_seq); DECL_ANCHOR(cond_seq); int only_special_literals = 1; - VALUE literals = rb_hash_new_with_size_and_type(0, 0, &cdhash_type); + VALUE literals = cdhash_new(0); int line; enum node_type type; const NODE *line_node; @@ -12166,7 +12192,7 @@ iseq_build_from_ary_body(rb_iseq_t *iseq, LINK_ANCHOR *const anchor, case TS_CDHASH: { int i; - VALUE map = rb_hash_new_with_size_and_type(0, RARRAY_LEN(op)/2, &cdhash_type); + VALUE map = cdhash_new(RARRAY_LEN(op) / 2); op = rb_to_array_type(op); for (i=0; inum_entries; + ibf_dump_write_small_value(dump, (VALUE)len); + if (len > 0) st_foreach(tbl, ibf_dump_object_hash_i, (VALUE)dump); + break; + } + default: + ibf_dump_object_unsupported(dump, obj); + break; + } +} + +static VALUE +ibf_load_object_imemo(const struct ibf_load *load, const struct ibf_object_header *header, ibf_offset_t offset) +{ + long len = (long)ibf_load_small_value(load, &offset); + VALUE obj = cdhash_new(len); + + int i; + for (i = 0; i < len; i++) { + VALUE key_index = ibf_load_small_value(load, &offset); + VALUE val_index = ibf_load_small_value(load, &offset); + + VALUE key = ibf_load_object(load, key_index); + VALUE val = ibf_load_object(load, val_index); + cdhash_aset(obj, key, val); + } + + return obj; +} + static void ibf_dump_object_struct(struct ibf_dump *dump, VALUE obj) { @@ -14531,7 +14577,7 @@ static const ibf_dump_object_function dump_object_functions[RUBY_T_MASK+1] = { ibf_dump_object_unsupported, /* 0x17 */ ibf_dump_object_unsupported, /* 0x18 */ ibf_dump_object_unsupported, /* 0x19 */ - ibf_dump_object_unsupported, /* T_IMEMO 0x1a */ + ibf_dump_object_imemo, /* T_IMEMO 0x1a */ ibf_dump_object_unsupported, /* T_NODE 0x1b */ ibf_dump_object_unsupported, /* T_ICLASS 0x1c */ ibf_dump_object_unsupported, /* T_ZOMBIE 0x1d */ @@ -14624,7 +14670,7 @@ static const ibf_load_object_function load_object_functions[RUBY_T_MASK+1] = { ibf_load_object_unsupported, /* 0x17 */ ibf_load_object_unsupported, /* 0x18 */ ibf_load_object_unsupported, /* 0x19 */ - ibf_load_object_unsupported, /* T_IMEMO 0x1a */ + ibf_load_object_imemo, /* T_IMEMO 0x1a */ ibf_load_object_unsupported, /* T_NODE 0x1b */ ibf_load_object_unsupported, /* T_ICLASS 0x1c */ ibf_load_object_unsupported, /* T_ZOMBIE 0x1d */ diff --git a/debug_counter.h b/debug_counter.h index 70437b68d80..721ff9d1b8a 100644 --- a/debug_counter.h +++ b/debug_counter.h @@ -316,6 +316,7 @@ RB_DEBUG_COUNTER(obj_imemo_callcache) RB_DEBUG_COUNTER(obj_imemo_constcache) RB_DEBUG_COUNTER(obj_imemo_fields) RB_DEBUG_COUNTER(obj_imemo_subclasses) +RB_DEBUG_COUNTER(obj_imemo_cdhash) RB_DEBUG_COUNTER(opt_new_hit) RB_DEBUG_COUNTER(opt_new_miss) diff --git a/ext/objspace/objspace.c b/ext/objspace/objspace.c index 0f299c2ab3c..38bffb07f70 100644 --- a/ext/objspace/objspace.c +++ b/ext/objspace/objspace.c @@ -460,6 +460,7 @@ count_imemo_objects(int argc, VALUE *argv, VALUE self) INIT_IMEMO_TYPE_ID(imemo_constcache); INIT_IMEMO_TYPE_ID(imemo_fields); INIT_IMEMO_TYPE_ID(imemo_subclasses); + INIT_IMEMO_TYPE_ID(imemo_cdhash); #undef INIT_IMEMO_TYPE_ID } diff --git a/gc.c b/gc.c index e269cb0fdbf..3c5af0412ac 100644 --- a/gc.c +++ b/gc.c @@ -1350,6 +1350,7 @@ rb_gc_imemo_needs_cleanup_p(VALUE obj) case imemo_ment: case imemo_iseq: case imemo_callinfo: + case imemo_cdhash: return true; case imemo_subclasses: diff --git a/hash.c b/hash.c index c6ce7b2a053..fbad278bee8 100644 --- a/hash.c +++ b/hash.c @@ -1469,14 +1469,6 @@ rb_hash_new_with_size(st_index_t size) return ret; } -VALUE -rb_hash_new_with_size_and_type(VALUE klass, st_index_t size, const struct st_hash_type *type) -{ - VALUE ret = hash_alloc_flags(klass, 0, Qnil, true); - hash_st_table_init(ret, type, size); - return ret; -} - VALUE rb_hash_new_capa(long capa) { diff --git a/imemo.c b/imemo.c index 7f4294381de..5ad8773b27d 100644 --- a/imemo.c +++ b/imemo.c @@ -32,6 +32,7 @@ rb_imemo_name(enum imemo_type type) IMEMO_NAME(cvar_entry); IMEMO_NAME(fields); IMEMO_NAME(subclasses); + IMEMO_NAME(cdhash); #undef IMEMO_NAME } rb_bug("unreachable"); @@ -124,6 +125,14 @@ rb_imemo_memo_new_value(VALUE a, VALUE b, VALUE c) return memo; } +VALUE +rb_imemo_cdhash_new(size_t size, const struct st_hash_type *type) +{ + struct rb_imemo_cdhash *memo = IMEMO_NEW(struct rb_imemo_cdhash, imemo_cdhash, 0); + st_init_existing_table_with_size(&memo->tbl, type, size); + return (VALUE)memo; +} + VALUE rb_imemo_fields_new(VALUE owner, shape_id_t shape_id, bool shareable) { @@ -289,14 +298,20 @@ rb_imemo_memsize(VALUE obj) if (rb_obj_shape_complex_p(obj)) { size += st_memsize(IMEMO_OBJ_FIELDS(obj)->as.complex.table); } + break; case imemo_subclasses: { if (FL_TEST_RAW(obj, IMEMO_SUBCLASSES_HEAP)) { struct rb_subclasses *subs = (struct rb_subclasses *)obj; size += subs->capacity * sizeof(VALUE); } + break; } + case imemo_cdhash: + size += st_memsize(rb_imemo_cdhash_tbl(obj)) - sizeof(st_table); + + break; default: rb_bug("unreachable"); } @@ -578,6 +593,16 @@ rb_imemo_mark_and_move(VALUE obj, bool reference_updating) } break; } + case imemo_cdhash: { + st_table *tbl = rb_imemo_cdhash_tbl(obj); + if (reference_updating) { + rb_gc_update_tbl_refs(tbl); + } + else { + rb_mark_tbl_no_pin(tbl); + } + break; + } default: rb_bug("unreachable"); } @@ -686,6 +711,7 @@ rb_imemo_free(VALUE obj) case imemo_fields: imemo_fields_free(IMEMO_OBJ_FIELDS(obj)); RB_DEBUG_COUNTER_INC(obj_imemo_fields); + break; case imemo_subclasses: { if (FL_TEST_RAW(obj, IMEMO_SUBCLASSES_HEAP)) { @@ -695,6 +721,11 @@ rb_imemo_free(VALUE obj) RB_DEBUG_COUNTER_INC(obj_imemo_subclasses); break; } + case imemo_cdhash: + st_free_embedded_table(rb_imemo_cdhash_tbl(obj)); + RB_DEBUG_COUNTER_INC(obj_imemo_cdhash); + + break; default: rb_bug("unreachable"); } diff --git a/internal/hash.h b/internal/hash.h index 8c5511e82ba..6671cd496d1 100644 --- a/internal/hash.h +++ b/internal/hash.h @@ -112,7 +112,6 @@ int rb_hash_stlike_foreach(VALUE hash, st_foreach_callback_func *func, st_data_t RUBY_SYMBOL_EXPORT_END VALUE rb_hash_new_with_size(st_index_t size); -VALUE rb_hash_new_with_size_and_type(VALUE klass, st_index_t size, const struct st_hash_type *type); VALUE rb_hash_resurrect(VALUE hash); int rb_hash_stlike_lookup(VALUE hash, st_data_t key, st_data_t *pval); VALUE rb_hash_keys(VALUE hash); diff --git a/internal/imemo.h b/internal/imemo.h index e185ad602d4..e8a5f0fc8eb 100644 --- a/internal/imemo.h +++ b/internal/imemo.h @@ -43,6 +43,7 @@ enum imemo_type { imemo_constcache = 12, imemo_fields = 13, imemo_subclasses = 14, + imemo_cdhash = 15, }; /* CREF (Class REFerence) is defined in method.h */ @@ -98,6 +99,11 @@ struct rb_imemo_tmpbuf_struct { size_t size; /* buffer size in bytes */ }; +struct rb_imemo_cdhash { + VALUE flags; + st_table tbl; +}; + /* Set on imemo_memo when u3 holds a VALUE that GC must mark. * When unset, u3 is a non-VALUE (cnt/state). */ #define MEMO_U3_IS_VALUE IMEMO_FL_USER0 @@ -223,6 +229,15 @@ MEMO_V2_SET(struct MEMO *m, VALUE v) RB_OBJ_WRITE(m, &m->v2, v); } +VALUE rb_imemo_cdhash_new(size_t size, const struct st_hash_type *type); + +static inline st_table * +rb_imemo_cdhash_tbl(VALUE obj) +{ + RUBY_ASSERT(IMEMO_TYPE_P(obj, imemo_cdhash)); + return &((struct rb_imemo_cdhash *)obj)->tbl; +} + struct rb_fields { struct RBasic basic; union { diff --git a/prism_compile.c b/prism_compile.c index 0f86ee9027f..c5fed3f532c 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -7606,9 +7606,7 @@ pm_compile_case_node_dispatch(rb_iseq_t *iseq, VALUE dispatch, const pm_node_t * return Qundef; } - if (NIL_P(rb_hash_lookup(dispatch, key))) { - rb_hash_aset(dispatch, key, ((VALUE) label) | 1); - } + cdhash_aset_if_missing(dispatch, key, ((VALUE) label) | 1); return dispatch; } @@ -7747,8 +7745,7 @@ pm_compile_case_node(rb_iseq_t *iseq, const pm_case_node_t *cast, const pm_node_ // lookup to jump directly to the correct when clause body. VALUE dispatch = Qundef; if (ISEQ_COMPILE_DATA(iseq)->option->specialized_instruction) { - dispatch = rb_hash_new(); - RHASH_TBL_RAW(dispatch)->type = &cdhash_type; + dispatch = cdhash_new(0); } // We're going to loop through each of the conditions in the case diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 92091216f41..aed1840c12c 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -6577,7 +6577,7 @@ vm_case_dispatch(CDHASH hash, OFFSET else_offset, VALUE key) key = FIXABLE(kval) ? LONG2FIX((long)kval) : rb_dbl2big(kval); } } - if (rb_hash_stlike_lookup(hash, key, &val)) { + if (st_lookup(rb_imemo_cdhash_tbl(hash), key, &val)) { return FIX2LONG((VALUE)val); } else { diff --git a/yjit.c b/yjit.c index cf8b58daa52..99375a1cdac 100644 --- a/yjit.c +++ b/yjit.c @@ -509,6 +509,30 @@ rb_vm_instruction_size(void) return VM_INSTRUCTION_SIZE; } +static int +yjit_cdhash_all_fixnum_i(st_data_t key, st_data_t _val, st_data_t data) +{ + if (!FIXNUM_P((VALUE)key)) { + *((bool *)data) = false; + return ST_STOP; + } + return ST_CONTINUE; +} + +bool +rb_yjit_cdhash_all_fixnum_p(VALUE cdhash) +{ + bool all_fixnum = true; + st_foreach(rb_imemo_cdhash_tbl(cdhash), yjit_cdhash_all_fixnum_i, (st_data_t)&all_fixnum); + return all_fixnum; +} + +int +rb_yjit_cdhash_lookup(VALUE cdhash, st_data_t key, st_data_t *val) +{ + return st_lookup(rb_imemo_cdhash_tbl(cdhash), key, val); +} + // Primitives used by yjit.rb VALUE rb_yjit_stats_enabled_p(rb_execution_context_t *ec, VALUE self); VALUE rb_yjit_print_stats_p(rb_execution_context_t *ec, VALUE self); diff --git a/yjit/bindgen/src/main.rs b/yjit/bindgen/src/main.rs index 73e9c847465..a6a24387b3b 100644 --- a/yjit/bindgen/src/main.rs +++ b/yjit/bindgen/src/main.rs @@ -99,7 +99,6 @@ fn main() { .allowlist_function("rb_hash_new") .allowlist_function("rb_hash_new_with_size") .allowlist_function("rb_hash_resurrect") - .allowlist_function("rb_hash_stlike_foreach") .allowlist_function("rb_to_hash_type") .allowlist_type("st_retval") .allowlist_function("rb_hash_aset") @@ -284,6 +283,8 @@ fn main() { .allowlist_function("rb_jit_for_each_iseq") .allowlist_type("jit_bindgen_constants") .allowlist_function("rb_vm_barrier") + .allowlist_function("rb_yjit_cdhash_all_fixnum_p") + .allowlist_function("rb_yjit_cdhash_lookup") // Not sure why it's picking these up, but don't. .blocklist_type("FILE") diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 98b659721c6..88ad45b00e2 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -4651,21 +4651,8 @@ fn gen_opt_case_dispatch( // Check that all cases are fixnums to avoid having to register BOP assumptions on // all the types that case hashes support. This spends compile time to save memory. - fn case_hash_all_fixnum_p(hash: VALUE) -> bool { - let mut all_fixnum = true; - unsafe { - unsafe extern "C" fn per_case(key: st_data_t, _value: st_data_t, data: st_data_t) -> c_int { - (if VALUE(key as usize).fixnum_p() { - ST_CONTINUE - } else { - (data as *mut bool).write(false); - ST_STOP - }) as c_int - } - rb_hash_stlike_foreach(hash, Some(per_case), (&mut all_fixnum) as *mut _ as st_data_t); - } - - all_fixnum + fn case_hash_all_fixnum_p(cdhash: VALUE) -> bool { + unsafe { rb_yjit_cdhash_all_fixnum_p(cdhash) } } // If megamorphic, fallback to compiling branch instructions after opt_case_dispatch @@ -4692,8 +4679,7 @@ fn gen_opt_case_dispatch( // Get the offset for the compile-time key let mut offset = 0; - unsafe { rb_hash_stlike_lookup(case_hash, comptime_key.0 as _, &mut offset) }; - let jump_offset = if offset == 0 { + let jump_offset = if unsafe { rb_yjit_cdhash_lookup(case_hash, comptime_key.0 as _, &mut offset) } == 0 { // NOTE: If we hit the else branch with various values, it could negatively impact the performance. else_offset } else { diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index 16045f7e8b8..392e393378b 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -264,13 +264,6 @@ pub const ST_DELETE: st_retval = 2; pub const ST_CHECK: st_retval = 3; pub const ST_REPLACE: st_retval = 4; pub type st_retval = u32; -pub type st_foreach_callback_func = ::std::option::Option< - unsafe extern "C" fn( - arg1: st_data_t, - arg2: st_data_t, - arg3: st_data_t, - ) -> ::std::os::raw::c_int, ->; pub const RARRAY_EMBED_FLAG: ruby_rarray_flags = 8192; pub const RARRAY_EMBED_LEN_MASK: ruby_rarray_flags = 4161536; pub type ruby_rarray_flags = u32; @@ -362,6 +355,7 @@ pub const imemo_callcache: imemo_type = 11; pub const imemo_constcache: imemo_type = 12; pub const imemo_fields: imemo_type = 13; pub const imemo_subclasses: imemo_type = 14; +pub const imemo_cdhash: imemo_type = 15; pub type imemo_type = u32; #[repr(C)] #[derive(Debug, Copy, Clone)] @@ -1141,11 +1135,6 @@ extern "C" { chilled: bool, ) -> VALUE; pub fn rb_to_hash_type(obj: VALUE) -> VALUE; - pub fn rb_hash_stlike_foreach( - hash: VALUE, - func: st_foreach_callback_func, - arg: st_data_t, - ) -> ::std::os::raw::c_int; pub fn rb_hash_new_with_size(size: st_index_t) -> VALUE; pub fn rb_hash_resurrect(hash: VALUE) -> VALUE; pub fn rb_hash_stlike_lookup( @@ -1218,6 +1207,12 @@ extern "C" { leave_exception: *mut ::std::os::raw::c_void, ); pub fn rb_vm_instruction_size() -> u32; + pub fn rb_yjit_cdhash_all_fixnum_p(cdhash: VALUE) -> bool; + pub fn rb_yjit_cdhash_lookup( + cdhash: VALUE, + key: st_data_t, + val: *mut st_data_t, + ) -> ::std::os::raw::c_int; pub fn rb_iseq_encoded_size(iseq: *const rb_iseq_t) -> ::std::os::raw::c_uint; pub fn rb_iseq_pc_at_idx(iseq: *const rb_iseq_t, insn_idx: u32) -> *mut VALUE; pub fn rb_iseq_opcode_at_pc(iseq: *const rb_iseq_t, pc: *const VALUE) -> ::std::os::raw::c_int; diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index 1194a590df1..860e1726dbe 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -446,6 +446,7 @@ pub const imemo_callcache: imemo_type = 11; pub const imemo_constcache: imemo_type = 12; pub const imemo_fields: imemo_type = 13; pub const imemo_subclasses: imemo_type = 14; +pub const imemo_cdhash: imemo_type = 15; pub type imemo_type = u32; #[repr(C)] #[derive(Debug, Copy, Clone)]