diff --git a/gc.c b/gc.c index d21019dbab2..5101ba902f3 100644 --- a/gc.c +++ b/gc.c @@ -3699,10 +3699,17 @@ rb_gc_obj_optimal_size(VALUE obj) case T_HASH: { - if (RB_OBJ_FROZEN(obj) && RHASH_AR_TABLE_P(obj)) { - return sizeof(struct RHash) + offsetof(ar_table, pairs) + RHASH_AR_TABLE_BOUND(obj) * sizeof(ar_table_pair); + const size_t st_size = sizeof(struct RHash) + sizeof(st_table); + if (RHASH_ST_TABLE_P(obj)) { + return st_size; } - return sizeof(struct RHash) + (RHASH_ST_TABLE_P(obj) ? sizeof(st_table) : sizeof(ar_table)); + + const size_t ar_size = sizeof(struct RHash) + offsetof(ar_table, pairs) + RHASH_AR_TABLE_BOUND(obj) * sizeof(ar_table_pair); + if (OBJ_FROZEN(obj) || ar_size > st_size) { + return ar_size; + } + + return st_size; } default: diff --git a/hash.c b/hash.c index fe1a8bbb7fb..37fad66abca 100644 --- a/hash.c +++ b/hash.c @@ -412,7 +412,26 @@ typedef st_index_t st_hash_t; static inline unsigned int RHASH_AR_TABLE_MAX_BOUND(VALUE h) { - return RHASH_AR_TABLE_MAX_SIZE; + size_t usable_space = rb_obj_shape_slot_size(h) - sizeof(struct RHash) - offsetof(ar_table, pairs); + usable_space /= sizeof(ar_table_pair); +#if SIZEOF_VALUE == 8 + RBIMPL_ASSERT_OR_ASSUME(usable_space <= RHASH_AR_TABLE_MAX_SIZE); + return (unsigned)usable_space; +#else + return usable_space <= RHASH_AR_TABLE_MAX_SIZE ? (unsigned)usable_space : RHASH_AR_TABLE_MAX_SIZE; +#endif +} + +static inline size_t +ar_table_memsize(size_t capa) +{ + return offsetof(ar_table, pairs) + capa * sizeof(ar_table_pair); +} + +static inline size_t +ar_memsize(size_t capa) +{ + return sizeof(struct RHash) + ar_table_memsize(capa); } #define RHASH_AR_TABLE_CONVERTED_TO_ST_TABLE (RHASH_AR_TABLE_MAX_SIZE + 1) @@ -539,6 +558,7 @@ RHASH_TABLE_EMPTY_P(VALUE hash) static void hash_st_table_init(VALUE hash, const struct st_hash_type *type, st_index_t size) { + RUBY_ASSERT(rb_gc_obj_slot_size(hash) >= sizeof(struct RHash) + sizeof(st_table)); st_init_existing_table_with_size(RHASH_ST_TABLE(hash), type, size); RHASH_SET_ST_FLAG(hash); } @@ -603,11 +623,10 @@ RHASH_AR_TABLE_SIZE_DEC(VALUE h) static inline void RHASH_AR_TABLE_CLEAR(VALUE h) { - RUBY_ASSERT(rb_gc_obj_slot_size(h) >= sizeof(struct RHash) + sizeof(ar_table)); RBASIC(h)->flags &= ~RHASH_AR_TABLE_SIZE_MASK; RBASIC(h)->flags &= ~RHASH_AR_TABLE_BOUND_MASK; - memset(RHASH_AR_TABLE(h), 0, sizeof(ar_table)); + memset(RHASH_AR_TABLE(h), 0, rb_obj_shape_slot_size(h) - sizeof(struct RHash)); } NOINLINE(static int ar_equal(VALUE x, VALUE y)); @@ -757,7 +776,7 @@ ar_force_convert_table(VALUE hash, const char *file, int line) st_hash_t hashes[RHASH_AR_TABLE_MAX_SIZE]; unsigned int bound, size; - RUBY_ASSERT(rb_gc_obj_slot_size(hash) >= sizeof(struct RHash) + sizeof(ar_table)); + RUBY_ASSERT(rb_gc_obj_slot_size(hash) >= sizeof(struct RHash) + sizeof(st_table)); // prepare hash values while (1) { @@ -790,6 +809,28 @@ ar_force_convert_table(VALUE hash, const char *file, int line) } } +static void +ar_compact_into(VALUE dst, VALUE src) +{ + ar_table_pair *dst_pairs = RHASH_AR_TABLE(dst)->pairs; + ar_table_pair *src_pairs = RHASH_AR_TABLE(src)->pairs; + + const unsigned src_bound = RHASH_AR_TABLE_BOUND(src); + const unsigned src_size = RHASH_AR_TABLE_SIZE(src); + + unsigned j=0; + for (unsigned i = 0; i < src_bound; i++) { + if (!ar_cleared_entry(src, i)) { + dst_pairs[j] = src_pairs[i]; + ar_hint_set_hint(dst, j, (st_hash_t)ar_hint(src, i)); + j++; + } + } + RHASH_AR_TABLE_BOUND_SET(dst, src_size); + RHASH_AR_TABLE_SIZE_SET(dst, src_size); + hash_verify(dst); +} + static int ar_compact_table(VALUE hash) { @@ -1040,11 +1081,9 @@ ar_update(VALUE hash, st_data_t key, } static int -ar_insert(VALUE hash, st_data_t key, st_data_t value) +ar_insert_direct(VALUE hash, st_data_t key, st_data_t value, st_hash_t hash_value) { unsigned bin = RHASH_AR_TABLE_BOUND(hash); - st_hash_t hash_value = ar_do_hash(key); - if (UNLIKELY(!RHASH_AR_TABLE_P(hash))) { // `#hash` changes ar_table -> st_table return -1; @@ -1074,6 +1113,13 @@ ar_insert(VALUE hash, st_data_t key, st_data_t value) } } +static int +ar_insert(VALUE hash, st_data_t key, st_data_t value) +{ + st_hash_t hash_value = ar_do_hash(key); + return ar_insert_direct(hash, key, value, hash_value); +} + static int ar_lookup(VALUE hash, st_data_t key, st_data_t *value) { @@ -1199,16 +1245,21 @@ ar_values(VALUE hash, st_data_t *values, st_index_t size) static ar_table* ar_copy(VALUE hash1, VALUE hash2) { - RUBY_ASSERT(rb_gc_obj_slot_size(hash1) >= sizeof(struct RHash) + sizeof(ar_table)); - ar_table *old_tab = RHASH_AR_TABLE(hash2); + RUBY_ASSERT(rb_gc_obj_slot_size(hash1) >= ar_memsize(RHASH_SIZE(hash2))); ar_table *new_tab = RHASH_AR_TABLE(hash1); unsigned int bound = RHASH_AR_TABLE_BOUND(hash2); + unsigned int size = RHASH_AR_TABLE_SIZE(hash2); + if (UNLIKELY(bound != size)) { + ar_compact_into(hash1, hash2); + return new_tab; + } + + ar_table *old_tab = RHASH_AR_TABLE(hash2); new_tab->ar_hint.word = old_tab->ar_hint.word; MEMCPY(&new_tab->pairs, &old_tab->pairs, ar_table_pair, bound); RHASH_AR_TABLE_BOUND_SET(hash1, bound); RHASH_AR_TABLE_SIZE_SET(hash1, RHASH_AR_TABLE_SIZE(hash2)); - rb_gc_writebarrier_remember(hash1); return new_tab; @@ -1490,16 +1541,19 @@ compact_after_delete(VALUE hash) static inline size_t hash_slot_size(size_t capa, bool frozen) { + const size_t st_size = sizeof(struct RHash) + sizeof(st_table); if (capa > RHASH_AR_TABLE_MAX_SIZE) { - return sizeof(struct RHash) + sizeof(st_table); + return st_size; } + const size_t ar_size = ar_memsize(capa); // If the hash is immutable, we can allocate a slot with exactly as much space as needed. - if (frozen) { - return sizeof(struct RHash) + offsetof(ar_table, pairs) + capa * sizeof(ar_table_pair); + // But if mutable, we must ensure we have enough space to transition to an st_table. + if (frozen || ar_size >= st_size) { + return ar_size; } - return sizeof(struct RHash) + sizeof(ar_table); + return st_size; } static VALUE @@ -1524,6 +1578,9 @@ hash_init_capa(VALUE hash, size_t size) if (size > RHASH_AR_TABLE_MAX_SIZE) { hash_st_table_init(hash, &objhash, size); } + else { + RUBY_ASSERT(RHASH_AR_TABLE_MAX_BOUND(hash) >= size); + } return hash; } @@ -1545,15 +1602,6 @@ rb_hash_alloc_copy(VALUE klass, VALUE src) return hash_alloc_capa(klass, RHASH_SIZE(src)); } -#if USE_ZJIT -size_t -rb_zjit_hash_new_size(VALUE *flags_out) -{ - *flags_out = T_HASH; - return hash_slot_size(0, false); -} -#endif - static VALUE empty_hash_alloc(VALUE klass) { @@ -1598,14 +1646,32 @@ rb_hash_alloc_fixed_size(VALUE klass, st_index_t size) return hash_init_capa(hash_alloc(klass, 0, Qnil, size, true), size); } +static int +ar_add_direct_i(st_data_t key, st_data_t value, st_data_t hash_value, st_data_t arg) +{ + VALUE ret = (VALUE)arg; + ar_insert_direct(ret, key, value, hash_value); + return ST_CONTINUE; +} + static VALUE hash_copy(VALUE ret, VALUE hash) { RUBY_ASSERT(RHASH_SIZE(ret) == 0); + if (RHASH_ST_TABLE_P(ret)) { + RUBY_ASSERT(RHASH_ST_TABLE(ret)->entries == NULL); + RHASH_UNSET_ST_FLAG(ret); + } if (rb_hash_compare_by_id_p(hash)) { + // If `hash` is an ar_table it can't be `compare_by_identity?`. + RUBY_ASSERT(RHASH_ST_TABLE_P(hash)); + RHASH_SET_ST_FLAG(ret); rb_gc_register_pinning_obj(ret); } + else if (RHASH_AR_TABLE_MAX_BOUND(ret) < RHASH_SIZE(hash)) { + RHASH_SET_ST_FLAG(ret); + } if (RHASH_AR_TABLE_P(hash)) { if (RHASH_AR_TABLE_P(ret)) { @@ -1613,11 +1679,7 @@ hash_copy(VALUE ret, VALUE hash) } else { st_table *tab = RHASH_ST_TABLE(ret); - - // If `hash` is an ar_table it can't be `compare_by_identity?`. - RUBY_ASSERT(!rb_hash_compare_by_id_p(hash)); - RUBY_ASSERT(RHASH_ST_TABLE(ret)->entries == NULL); - st_init_existing_table_with_size(RHASH_ST_TABLE(ret), &objhash, RHASH_SIZE(hash)); + st_init_existing_table_with_size(tab, &objhash, RHASH_SIZE(hash)); int bound = RHASH_AR_TABLE_BOUND(hash); for (int i = 0; i < bound; i++) { @@ -1631,9 +1693,13 @@ hash_copy(VALUE ret, VALUE hash) } } else { - RHASH_SET_ST_FLAG(ret); - st_replace(RHASH_ST_TABLE(ret), RHASH_ST_TABLE(hash)); - rb_gc_writebarrier_remember(ret); + if (RHASH_AR_TABLE_P(ret)) { + rb_st_foreach_with_hash(RHASH_ST_TABLE(hash), ar_add_direct_i, (st_data_t)ret); + } + else { + st_replace(RHASH_ST_TABLE(ret), RHASH_ST_TABLE(hash)); + rb_gc_writebarrier_remember(ret); + } } return ret; } @@ -1663,6 +1729,9 @@ hash_dup_capa(VALUE hash, size_t capa) if (capa > RHASH_AR_TABLE_MAX_SIZE) { RHASH_SET_ST_FLAG(ret); } + else { + RUBY_ASSERT(RHASH_AR_TABLE_MAX_BOUND(ret) >= capa); + } hash_copy(ret, hash); return ret; } @@ -1685,6 +1754,14 @@ rb_hash_resurrect(VALUE hash) } #if USE_ZJIT +size_t +rb_zjit_hash_new_size(VALUE *flags_out, size_t size) +{ + RUBY_ASSERT(size <= RHASH_AR_TABLE_MAX_SIZE); + *flags_out = T_HASH; + return hash_slot_size(size, false); +} + bool rb_zjit_hash_dup_can_fastpath(VALUE hash, size_t *alloc_size_out, VALUE *flags_out, VALUE *ifnone_out, long *bound_out) { @@ -1693,7 +1770,7 @@ rb_zjit_hash_dup_can_fastpath(VALUE hash, size_t *alloc_size_out, VALUE *flags_o const unsigned int bound = RHASH_AR_TABLE_BOUND(hash); - *alloc_size_out = hash_slot_size(0, false); + *alloc_size_out = hash_slot_size(bound, false); *flags_out = T_HASH | ((VALUE)RHASH_AR_TABLE_SIZE(hash) << RHASH_AR_TABLE_SIZE_SHIFT) | ((VALUE)bound << RHASH_AR_TABLE_BOUND_SHIFT); @@ -1859,7 +1936,7 @@ rb_hash_init(rb_execution_context_t *ec, VALUE hash, VALUE capa_value, VALUE ifn if (capa_value != INT2FIX(0)) { long capa = NUM2LONG(capa_value); - if (capa > RHASH_AR_TABLE_MAX_SIZE && RHASH_SIZE(hash) == 0 && RHASH_AR_TABLE_P(hash)) { + if (capa > 0 && RHASH_AR_TABLE_P(hash) && RHASH_SIZE(hash) == 0 && capa > RHASH_AR_TABLE_MAX_BOUND(hash)) { hash_st_table_init(hash, &objhash, capa); } } @@ -5248,7 +5325,7 @@ rb_hash_bulk_insert(long argc, const VALUE *argv, VALUE hash) st_index_t size = argc / 2; if (RHASH_AR_TABLE_P(hash) && - (RHASH_AR_TABLE_SIZE(hash) + size <= RHASH_AR_TABLE_MAX_SIZE)) { + (RHASH_AR_TABLE_SIZE(hash) + size <= RHASH_AR_TABLE_MAX_BOUND(hash))) { ar_bulk_insert(hash, argc, argv); } else { diff --git a/internal/hash.h b/internal/hash.h index 3f7331ab265..f698f9f5707 100644 --- a/internal/hash.h +++ b/internal/hash.h @@ -89,6 +89,7 @@ bool rb_hash_default_unredefined(VALUE hash); VALUE rb_hash_alloc_fixed_size(VALUE klass, st_index_t size); VALUE rb_ident_hash_new_capa(long size); void rb_hash_free(VALUE hash); +VALUE rb_hash_alloc_copy(VALUE klass, VALUE src); RUBY_EXTERN VALUE rb_cHash_empty_frozen; static inline unsigned RHASH_AR_TABLE_SIZE_RAW(VALUE h); diff --git a/internal/st.h b/internal/st.h index 44885c09b92..bd666bb39b6 100644 --- a/internal/st.h +++ b/internal/st.h @@ -20,4 +20,7 @@ void rb_st_free_embedded_table(st_table *tab); int rb_st_insert_no_rebuild(st_table *tab, st_data_t key, st_data_t value); #define st_insert_no_rebuild rb_st_insert_no_rebuild +typedef int st_foreach_with_hash_callback_func(st_data_t, st_data_t, st_data_t, st_data_t); +int rb_st_foreach_with_hash(st_table *, st_foreach_with_hash_callback_func *, st_data_t); +#define st_foreach_with_hash rb_st_foreach_with_hash #endif diff --git a/object.c b/object.c index 27b8654a738..6327f4aadc4 100644 --- a/object.c +++ b/object.c @@ -28,6 +28,7 @@ #include "internal/eval.h" #include "internal/hash.h" #include "internal/inits.h" +#include "internal/hash.h" #include "internal/numeric.h" #include "internal/object.h" #include "internal/struct.h" @@ -612,7 +613,16 @@ rb_obj_dup(VALUE obj) if (special_object_p(obj)) { return obj; } - dup = rb_obj_alloc(rb_obj_class(obj)); + + switch (OBJ_BUILTIN_TYPE(obj)) { + case T_HASH: + dup = rb_hash_alloc_copy(rb_obj_class(obj), obj); + break; + default: + dup = rb_obj_alloc(rb_obj_class(obj)); + break; + } + return rb_obj_dup_setup(obj, dup); } diff --git a/st.c b/st.c index f7b870fece4..3fa125bbf72 100644 --- a/st.c +++ b/st.c @@ -1719,6 +1719,57 @@ st_general_foreach(st_table *tab, st_foreach_check_callback_func *func, st_updat return 0; } +#ifdef INTERNAL_ST_H +int +st_foreach_with_hash(st_table *tab, st_foreach_with_hash_callback_func *func, st_data_t arg) +{ + st_table_entry *entries, *curr_entry_ptr; + enum st_retval retval; + st_index_t i, rebuilds_num; + st_hash_t hash; + st_data_t key; + int packed_p = !st_has_bins(tab); + + entries = tab->entries; + /* The bound can change inside the loop even without rebuilding + the table, e.g. by an entry insertion. */ + for (i = tab->entries_start; i < tab->entries_bound; i++) { + curr_entry_ptr = &entries[i]; + if (EXPECT(DELETED_ENTRY_P(curr_entry_ptr), 0)) + continue; + key = curr_entry_ptr->key; + rebuilds_num = tab->rebuilds_num; + hash = curr_entry_ptr->hash; + retval = (*func)(key, curr_entry_ptr->record, hash, arg); + + if (rebuilds_num != tab->rebuilds_num) { + retry: + entries = tab->entries; + packed_p = !st_has_bins(tab); + if (packed_p) { + i = find_entry(tab, hash, key); + if (EXPECT(i == REBUILT_TABLE_ENTRY_IND, 0)) + goto retry; + } + else { + i = find_table_entry_ind(tab, hash, key); + if (EXPECT(i == REBUILT_TABLE_ENTRY_IND, 0)) + goto retry; + i -= ENTRY_BASE; + } + curr_entry_ptr = &entries[i]; + } + switch (retval) { + case ST_STOP: + return 0; + default: + break; + } + } + return 0; +} +#endif + int st_foreach_with_replace(st_table *tab, st_foreach_check_callback_func *func, st_update_callback_func *replace, st_data_t arg) { diff --git a/test/ruby/test_hash.rb b/test/ruby/test_hash.rb index 6b4c72db7fb..109b6334bee 100644 --- a/test/ruby/test_hash.rb +++ b/test/ruby/test_hash.rb @@ -1958,10 +1958,23 @@ class TestHashOnly < Test::Unit::TestCase end end obj.hash_calls = 0 - hash = {obj => 42} + + ar_hash = {obj => 42} assert_equal(1, obj.hash_calls) - yield hash + yield ar_hash assert_equal(1, obj.hash_calls) + + st_hash = {a:1, b:2, c:3, d:4, e:5, f:6, g:7, h:8, obj => 42} + assert_equal(2, obj.hash_calls) + yield st_hash + assert_equal(2, obj.hash_calls) + + st_hash.keys.first(8).each do |key| + st_hash.delete(key) + end + assert_equal(2, obj.hash_calls) + yield st_hash + assert_equal(2, obj.hash_calls) end def test_select_reject_will_not_rehash diff --git a/zjit.h b/zjit.h index 8757c4d386f..7bda68202d4 100644 --- a/zjit.h +++ b/zjit.h @@ -132,7 +132,7 @@ void rb_zjit_invalidate_root_box(void); void rb_zjit_jit_frame_update_references(zjit_jit_frame_t *jit_frame); void rb_zjit_materialize_frames(const rb_execution_context_t *ec, rb_control_frame_t *cfp); void rb_zjit_materialize_frames_for_longjmp(const rb_execution_context_t *ec, rb_control_frame_t *cfp); -size_t rb_zjit_hash_new_size(VALUE *flags_out); +size_t rb_zjit_hash_new_size(VALUE *flags_out, size_t size); VALUE rb_zjit_new_obj_shape(VALUE flags, size_t alloc_size); bool rb_zjit_class_allocate_instance_fastpath(VALUE klass, size_t *size_out, VALUE *flags_out); bool rb_zjit_str_resurrect_fastpath(VALUE str, bool chilled, size_t *size_out, VALUE *flags_out, long *len_out, size_t *byte_size_out); diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 170f597735f..01d355eb349 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -2546,7 +2546,7 @@ fn gen_new_hash( gen_prepare_leaf_call_with_gc(asm, state); let mut flags = VALUE(0); - let alloc_size = unsafe { rb_zjit_hash_new_size(&mut flags) }; + let alloc_size = unsafe { rb_zjit_hash_new_size(&mut flags, 0) }; let klass = unsafe { rb_cHash }; gc_fastpath::gc_fastpath_new_obj(jit, asm, function, state, alloc_size, flags.into(), klass, @@ -2565,7 +2565,7 @@ fn gen_new_hash( let num_pairs = elements.len() / 2; let hash = if num_pairs <= RUBY_RHASH_AR_TABLE_MAX_SIZE as usize { let mut flags = VALUE(0); - let alloc_size = unsafe { rb_zjit_hash_new_size(&mut flags) }; + let alloc_size = unsafe { rb_zjit_hash_new_size(&mut flags, num_pairs) }; let klass = unsafe { rb_cHash }; gc_fastpath::gc_fastpath_new_obj(jit, asm, function, state, alloc_size, flags.into(), klass, diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index 68ff278b674..f85c7486035 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -2392,7 +2392,7 @@ unsafe extern "C" { pub fn rb_iseq_label(iseq: *const rb_iseq_t) -> VALUE; pub fn rb_iseq_defined_string(type_: defined_type) -> VALUE; pub fn rb_zjit_profile_enable(iseq: *const rb_iseq_t); - pub fn rb_zjit_hash_new_size(flags_out: *mut VALUE) -> usize; + pub fn rb_zjit_hash_new_size(flags_out: *mut VALUE, size: usize) -> usize; pub fn rb_zjit_new_obj_shape(flags: VALUE, alloc_size: usize) -> VALUE; pub fn rb_zjit_class_allocate_instance_fastpath( klass: VALUE,