diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index 22d2ee47deb1ce..f8c3573e1f24b5 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -2620,3 +2620,51 @@ def foo(a:, b:, c:) = super(a: a, b: b, c: c) :ok } + +# A thread terminating (here: the pipe writer) while another Ractor runs a +# compaction barrier must not corrupt the machine context of a thread blocked +# in IO, whose locked read buffer lives only on that machine stack. +assert_equal 'ok', %q{ + Warning[:experimental] = false + can_compact = begin + GC.compact + true + rescue NotImplementedError + false + end + if can_compact + b = Ractor.new do + 10.times do + r, w = IO.pipe + t = Thread.new { w.write("x" * 40); w.close } + r.read + r.close + t.join + end + :ok + end + a = Ractor.new { 20.times { GC.compact }; :ok } + [a, b].each(&:value) + end + :ok +} + +# Forking while other Ractors are alive takes a VM barrier in the parent; the +# child inherits that scheduler/barrier state and must reset it. Exercises the +# parent-side fork barrier and the child teardown, then checks the surviving +# Ractors still work. +assert_equal 'ok', %q{ + begin + rs = 5.times.map { Ractor.new { Ractor.receive } } + 10.times do + pid = fork { GC.start } + _, status = Process.waitpid2(pid) + raise "child failed" unless status.success? + end + rs.each { |r| r.send(nil) } + rs.each(&:value) + :ok + rescue NotImplementedError + :ok # platform without fork + end +} diff --git a/gc.c b/gc.c index 4bdc164347271b..650e4ea4ff0a57 100644 --- a/gc.c +++ b/gc.c @@ -376,7 +376,17 @@ rb_gc_shutdown_call_finalizer_p(VALUE obj) void rb_gc_obj_changed_slot_size(VALUE obj, size_t slot_size) { - RBASIC_SET_FULL_SHAPE_ID(obj, rb_obj_shape_transition_slot_size(obj, slot_size)); + shape_id_t shape_id = rb_obj_shape_transition_capacity(obj, rb_shape_capacity_for_slot_size(slot_size)); + + if (RB_TYPE_P(obj, T_OBJECT) && FL_TEST_RAW(obj, ROBJECT_HEAP) && !rb_obj_shape_complex_p(obj)) { + VALUE *embedded_fields = ROBJECT_EMBEDDED_FIELDS(obj); + VALUE *extended_fields = ROBJECT_FIELDS(obj); + MEMCPY(embedded_fields, extended_fields, VALUE, RSHAPE_LEN(RBASIC_SHAPE_ID(obj))); + FL_UNSET_RAW(obj, ROBJECT_HEAP); + shape_id = rb_shape_transition_robject(shape_id); + } + + RBASIC_SET_FULL_SHAPE_ID(obj, shape_id); } void rb_vm_update_references(void *ptr); @@ -1584,19 +1594,6 @@ rb_gc_obj_free(void *objspace, VALUE obj) switch (builtin_type) { case T_OBJECT: - if (FL_TEST_RAW(obj, ROBJECT_HEAP)) { - if (rb_obj_shape_complex_p(obj)) { - RB_DEBUG_COUNTER_INC(obj_obj_complex); - st_free_table(ROBJECT_FIELDS_HASH(obj)); - } - else { - SIZED_FREE_N(ROBJECT(obj)->as.heap.fields, ROBJECT_FIELDS_CAPACITY(obj)); - RB_DEBUG_COUNTER_INC(obj_obj_ptr); - } - } - else { - RB_DEBUG_COUNTER_INC(obj_obj_embed); - } break; case T_MODULE: case T_CLASS: @@ -2372,14 +2369,6 @@ rb_obj_memsize_of(VALUE obj) switch (BUILTIN_TYPE(obj)) { case T_OBJECT: - if (FL_TEST_RAW(obj, ROBJECT_HEAP)) { - if (rb_obj_shape_complex_p(obj)) { - size += rb_st_memsize(ROBJECT_FIELDS_HASH(obj)); - } - else { - size += ROBJECT_FIELDS_CAPACITY(obj) * sizeof(VALUE); - } - } break; case T_MODULE: case T_CLASS: @@ -3303,12 +3292,13 @@ rb_gc_mark_children(void *objspace, VALUE obj) case T_OBJECT: { uint32_t len; - if (rb_obj_shape_complex_p(obj)) { - gc_mark_tbl_no_pin(ROBJECT_FIELDS_HASH(obj)); - len = ROBJECT_FIELDS_COUNT_COMPLEX(obj); + if (FL_TEST_RAW(obj, ROBJECT_HEAP)) { + if (!rb_gc_checking_shareable()) { + gc_mark_internal(ROBJECT(obj)->as.extended); + } } else { - const VALUE * const ptr = ROBJECT_FIELDS(obj); + const VALUE * const ptr = ROBJECT(obj)->as.ary; len = ROBJECT_FIELDS_COUNT_NOT_COMPLEX(obj); for (uint32_t i = 0; i < len; i++) { @@ -3678,27 +3668,14 @@ gc_ref_update_array(void *objspace, VALUE v) static void gc_ref_update_object(void *objspace, VALUE v) { - VALUE *ptr = ROBJECT_FIELDS(v); - if (FL_TEST_RAW(v, ROBJECT_HEAP)) { - if (rb_obj_shape_complex_p(v)) { - gc_ref_update_table_values_only(ROBJECT_FIELDS_HASH(v)); - return; - } - - size_t slot_size = rb_gc_obj_slot_size(v); - size_t embed_size = rb_obj_embedded_size(ROBJECT_FIELDS_CAPACITY(v)); - if (slot_size >= embed_size) { - // Object can be re-embedded - memcpy(ROBJECT(v)->as.ary, ptr, sizeof(VALUE) * ROBJECT_FIELDS_COUNT(v)); - SIZED_FREE_N(ptr, ROBJECT_FIELDS_CAPACITY(v)); - FL_UNSET_RAW(v, ROBJECT_HEAP); - ptr = ROBJECT(v)->as.ary; - } + UPDATE_IF_MOVED(objspace, ROBJECT(v)->as.extended); } - - for (uint32_t i = 0; i < ROBJECT_FIELDS_COUNT(v); i++) { - UPDATE_IF_MOVED(objspace, ptr[i]); + else { + VALUE *ptr = ROBJECT_FIELDS(v); + for (uint32_t i = 0; i < ROBJECT_FIELDS_COUNT(v); i++) { + UPDATE_IF_MOVED(objspace, ptr[i]); + } } } @@ -4860,11 +4837,11 @@ rb_raw_obj_info_buitin_type(char *const buff, const size_t buff_size, const VALU APPEND_F("(complex) len:%zu", hash_len); } else { - APPEND_F("(embed) len:%d capa:%d", RSHAPE_LEN(RBASIC_SHAPE_ID(obj)), ROBJECT_FIELDS_CAPACITY(obj)); + APPEND_F("len:%d capa:%d extended:%p", RSHAPE_LEN(RBASIC_SHAPE_ID(obj)), ROBJECT_FIELDS_CAPACITY(obj), (void *)ROBJECT_FIELDS_OBJ(obj)); } } else { - APPEND_F("len:%d capa:%d ptr:%p", RSHAPE_LEN(RBASIC_SHAPE_ID(obj)), ROBJECT_FIELDS_CAPACITY(obj), (void *)ROBJECT_FIELDS(obj)); + APPEND_F("(embed) len:%d capa:%d", RSHAPE_LEN(RBASIC_SHAPE_ID(obj)), ROBJECT_FIELDS_CAPACITY(obj)); } } break; @@ -4889,6 +4866,21 @@ rb_raw_obj_info_buitin_type(char *const buff, const size_t buff_size, const VALU APPEND_F("<%s> ", rb_imemo_name(imemo_type(obj))); switch (imemo_type(obj)) { + case imemo_fields: + { + if (rb_obj_shape_complex_p(obj)) { + size_t hash_len = rb_st_table_size(rb_imemo_fields_complex_tbl(obj)); + APPEND_F("(complex) len:%zu", hash_len); + } + else { + APPEND_F("(embed) len:%d capa:%d", RSHAPE_LEN(RBASIC_SHAPE_ID(obj)), ROBJECT_FIELDS_CAPACITY(obj)); + } + + APPEND_S("owner -> "); + rb_raw_obj_info(BUFF_ARGS, CLASS_OF(obj)); + + break; + } case imemo_ment: { const rb_method_entry_t *me = (const rb_method_entry_t *)obj; diff --git a/imemo.c b/imemo.c index a70ae1a5523fcd..06a94d0d19a628 100644 --- a/imemo.c +++ b/imemo.c @@ -151,7 +151,6 @@ rb_imemo_fields_new(VALUE owner, shape_id_t shape_id, bool shareable) size_t embedded_size = offsetof(struct rb_fields, as.embed) + capa * sizeof(VALUE); VALUE fields = imemo_fields_new(owner, 0, shape_id, embedded_size, shareable); - RUBY_ASSERT(IMEMO_TYPE_P(fields, imemo_fields)); RUBY_ASSERT(rb_shape_embedded_capacity(RBASIC_SHAPE_ID(fields)) >= capa); @@ -223,14 +222,6 @@ rb_imemo_fields_clone(VALUE fields_obj) void rb_imemo_fields_clear(VALUE fields_obj) { - // When replacing an imemo/fields by another one, we must clear - // its shape so that gc.c:obj_free_object_id won't be called. - if (rb_obj_shape_complex_p(fields_obj)) { - RBASIC_SET_SHAPE_ID(fields_obj, ROOT_COMPLEX_SHAPE_ID); - } - else { - RBASIC_SET_SHAPE_ID(fields_obj, ROOT_SHAPE_ID); - } // Invalidate the ec->gen_fields_cache. RBASIC_CLEAR_CLASS(fields_obj); } diff --git a/include/ruby/internal/core/robject.h b/include/ruby/internal/core/robject.h index b8743dcb739116..c8917c0f8e2b19 100644 --- a/include/ruby/internal/core/robject.h +++ b/include/ruby/internal/core/robject.h @@ -89,23 +89,8 @@ struct RObject { /** Object's specific fields. */ union { - - /** - * Object that use separated memory region for instance variables use - * this pattern. - */ - struct { - /** Pointer to a C array that holds instance variables. */ - VALUE *fields; - } heap; - - /* When an object is too complex, it uses a st_table to store instance - * variable name to value mappings. - */ - st_table *hash; - - /* Embedded instance variables. When an object is small enough, it - * uses this area to store the instance variables. + /* Embedded instance variables. When an object slot is large enough, it + * uses this area to store the instance variables embedded. * * This is a length 1 array because: * 1. GCC has a bug that does not optimize C flexible array members @@ -113,6 +98,13 @@ struct RObject { * 2. Zero length arrays are not supported by all compilers */ VALUE ary[1]; + + /** + * When an object slot is too small or too complex to store instance + * variables inline, it references another, larger, object that contains + * the instance variables. + */ + VALUE extended; } as; }; diff --git a/internal/imemo.h b/internal/imemo.h index 7562794506f3d6..a66ed1161b424b 100644 --- a/internal/imemo.h +++ b/internal/imemo.h @@ -245,11 +245,7 @@ struct rb_fields { VALUE fields[1]; } embed; struct { - VALUE *ptr; - } external; - struct { - // Note: the st_table could be embedded, but complex T_CLASS should be rare to - // non-existent, so not really worth the trouble. + // TODO: the st_table should be embedded st_table *table; } complex; } as; @@ -260,8 +256,6 @@ struct rb_fields { #define OBJ_FIELD_HEAP ROBJECT_HEAP STATIC_ASSERT(imemo_fields_flags, OBJ_FIELD_HEAP == IMEMO_FL_USER0); STATIC_ASSERT(imemo_fields_embed_offset, offsetof(struct RObject, as.ary) == offsetof(struct rb_fields, as.embed.fields)); -STATIC_ASSERT(imemo_fields_external_offset, offsetof(struct RObject, as.heap.fields) == offsetof(struct rb_fields, as.external.ptr)); -STATIC_ASSERT(imemo_fields_complex_offset, offsetof(struct RObject, as.heap.fields) == offsetof(struct rb_fields, as.complex.table)); #define IMEMO_OBJ_FIELDS(fields) ((struct rb_fields *)fields) @@ -309,11 +303,18 @@ rb_imemo_fields_ptr(VALUE fields_obj) RUBY_ASSERT(IMEMO_TYPE_P(fields_obj, imemo_fields) || RB_TYPE_P(fields_obj, T_OBJECT)); if (UNLIKELY(FL_TEST_RAW(fields_obj, OBJ_FIELD_HEAP))) { - return IMEMO_OBJ_FIELDS(fields_obj)->as.external.ptr; - } - else { - return IMEMO_OBJ_FIELDS(fields_obj)->as.embed.fields; + if (RB_TYPE_P(fields_obj, T_OBJECT)) { + fields_obj = ROBJECT(fields_obj)->as.extended; + } + RUBY_ASSERT(IMEMO_TYPE_P(fields_obj, imemo_fields)); + + // This will go away once we embed the `st_table` inside `IMEMO/fields`. + if (UNLIKELY(FL_TEST_RAW(fields_obj, OBJ_FIELD_HEAP))) { + return (VALUE *)IMEMO_OBJ_FIELDS(fields_obj)->as.complex.table; + } } + + return IMEMO_OBJ_FIELDS(fields_obj)->as.embed.fields; } static inline st_table * @@ -323,7 +324,7 @@ rb_imemo_fields_complex_tbl(VALUE fields_obj) return NULL; } - RUBY_ASSERT(IMEMO_TYPE_P(fields_obj, imemo_fields) || RB_TYPE_P(fields_obj, T_OBJECT)); + RUBY_ASSERT(IMEMO_TYPE_P(fields_obj, imemo_fields)); RUBY_ASSERT(FL_TEST_RAW(fields_obj, OBJ_FIELD_HEAP)); // Some codepaths unconditionally access the fields_ptr, and assume it can be used as st_table if the diff --git a/internal/object.h b/internal/object.h index 7538d46b8b343e..2c531186ba3d75 100644 --- a/internal/object.h +++ b/internal/object.h @@ -68,19 +68,35 @@ RBASIC_SET_CLASS(VALUE obj, VALUE klass) RB_OBJ_WRITTEN(obj, oldv, klass); } +static inline VALUE +ROBJECT_FIELDS_OBJ(VALUE obj) +{ + RBIMPL_ASSERT_TYPE(obj, RUBY_T_OBJECT); + + return FL_TEST_RAW(obj, ROBJECT_HEAP) ? ROBJECT(obj)->as.extended : obj; +} + +static inline VALUE * +ROBJECT_EMBEDDED_FIELDS(VALUE obj) +{ + return ROBJECT(obj)->as.ary; +} + static inline VALUE * ROBJECT_FIELDS(VALUE obj) { RBIMPL_ASSERT_TYPE(obj, RUBY_T_OBJECT); - struct RObject *const ptr = ROBJECT(obj); + return ROBJECT_EMBEDDED_FIELDS(ROBJECT_FIELDS_OBJ(obj)); +} - if (RB_UNLIKELY(RB_FL_ANY_RAW(obj, ROBJECT_HEAP))) { - return ptr->as.heap.fields; - } - else { - return ptr->as.ary; - } +static inline void +ROBJECT_SET_EXTENDED(VALUE obj, VALUE fields_obj) +{ + RUBY_ASSERT(RB_TYPE_P(obj, T_OBJECT)); + RUBY_ASSERT(RB_TYPE_P(fields_obj, T_IMEMO)); + RB_OBJ_WRITE(obj, &ROBJECT(obj)->as.extended, fields_obj); + FL_SET_RAW(obj, ROBJECT_HEAP); } static inline size_t diff --git a/internal/variable.h b/internal/variable.h index 58364941c12156..70142815e89710 100644 --- a/internal/variable.h +++ b/internal/variable.h @@ -71,6 +71,5 @@ VALUE rb_gvar_get(ID); VALUE rb_gvar_set(ID, VALUE); VALUE rb_gvar_defined(ID); void rb_const_warn_if_deprecated(const rb_const_entry_t *, VALUE, ID); -void rb_ensure_iv_list_size(VALUE obj, uint32_t current_len, uint32_t newsize); #endif /* INTERNAL_VARIABLE_H */ diff --git a/jit.c b/jit.c index 534a07c848abfd..377ab3fe16177f 100644 --- a/jit.c +++ b/jit.c @@ -29,7 +29,7 @@ enum jit_bindgen_constants { // Field offsets for the RObject struct - ROBJECT_OFFSET_AS_HEAP_FIELDS = offsetof(struct RObject, as.heap.fields), + ROBJECT_OFFSET_AS_HEAP_FIELDS = offsetof(struct RObject, as.extended), ROBJECT_OFFSET_AS_ARY = offsetof(struct RObject, as.ary), // Field offset for prime classext's fields_obj from a class pointer diff --git a/lib/bundler/rubygems_gem_installer.rb b/lib/bundler/rubygems_gem_installer.rb index c6313ddf8d0eb7..d8c50556c531c8 100644 --- a/lib/bundler/rubygems_gem_installer.rb +++ b/lib/bundler/rubygems_gem_installer.rb @@ -94,6 +94,15 @@ def generate_plugins end end + # Reimplemented from RubyGems, since RubyGems older than 4.1 doesn't + # provide it + def remove_stale_plugins + return unless spec.plugins.empty? + + ensure_writable_dir @plugins_dir + remove_plugins_for(spec, @plugins_dir) + end + def warn_skipped_extensions return if spec.extensions.empty? diff --git a/lib/bundler/vendored_tsort.rb b/lib/bundler/vendored_tsort.rb index 8d8ab26ecef91c..d7d680344efd6e 100644 --- a/lib/bundler/vendored_tsort.rb +++ b/lib/bundler/vendored_tsort.rb @@ -1,8 +1,21 @@ # frozen_string_literal: true -begin - require "rubygems/vendored_tsort" -rescue LoadError - require "tsort" - Gem::TSort = TSort +# The defined? guard avoids reopening Gem::TSort when an old RubyGems has +# already loaded its own copy, e.g. through rubygems/request_set from +# Gem.activate_bin_path in binstubs. +# +unless defined?(Gem::TSort) + begin + require "rubygems/vendored_tsort" + rescue LoadError + begin + # RubyGems 3.4 and 3.5 ship the same file under its pre-3.6 name. + # Requiring the real tsort here instead would activate the tsort + # default gem, and `bundler/setup` must not activate any gems. + require "rubygems/tsort" + rescue LoadError + require "tsort" + Gem::TSort = TSort + end + end end diff --git a/object.c b/object.c index 919a2fd2e7cdec..3b382bcc5c0a7b 100644 --- a/object.c +++ b/object.c @@ -363,12 +363,17 @@ rb_obj_copy_ivar(VALUE dest, VALUE obj) RUBY_ASSERT(src_num_ivs <= dest_capa); if (initial_capa < dest_capa) { - rb_ensure_iv_list_size(dest, 0, dest_capa); + // We we need to transition the object to an extended layout. + VALUE fields_obj = rb_imemo_fields_new(dest, dest_shape_id, false); + ROBJECT_SET_EXTENDED(dest, fields_obj); dest_buf = ROBJECT_FIELDS(dest); + rb_shape_copy_fields(dest, dest_buf, dest_shape_id, src_buf, src_shape_id); + RBASIC_SET_SHAPE_ID_WITH_LAYOUT(dest, dest_shape_id, SHAPE_ID_LAYOUT_EXTENDED); + } + else { + rb_shape_copy_fields(dest, dest_buf, dest_shape_id, src_buf, src_shape_id); + RBASIC_SET_SHAPE_ID(dest, dest_shape_id); } - - rb_shape_copy_fields(dest, dest_buf, dest_shape_id, src_buf, src_shape_id); - RBASIC_SET_SHAPE_ID(dest, dest_shape_id); } static void diff --git a/ractor_sync.c b/ractor_sync.c index 5dfb86e3bff130..5b513a1315d831 100644 --- a/ractor_sync.c +++ b/ractor_sync.c @@ -126,7 +126,9 @@ ractor_port_receive(rb_execution_context_t *ec, VALUE self) rb_raise(rb_eRactorError, "only allowed from the creator Ractor of this port"); } - return ractor_receive(ec, rp); + VALUE v = ractor_receive(ec, rp); + RB_GC_GUARD(self); + return v; } static VALUE @@ -134,6 +136,7 @@ ractor_port_send(rb_execution_context_t *ec, VALUE self, VALUE obj, VALUE move) { const struct ractor_port *rp = RACTOR_PORT_PTR(self); ractor_send(ec, rp, obj, RTEST(move)); + RB_GC_GUARD(self); return self; } diff --git a/shape.c b/shape.c index 7bbf1e0328d380..8c1955fc939c43 100644 --- a/shape.c +++ b/shape.c @@ -1224,7 +1224,7 @@ rb_shape_expected_layout(VALUE obj) { switch (BUILTIN_TYPE(obj)) { case T_OBJECT: - return SHAPE_ID_LAYOUT_ROBJECT; + return FL_TEST_RAW(obj, ROBJECT_HEAP) ? SHAPE_ID_LAYOUT_EXTENDED : SHAPE_ID_LAYOUT_ROBJECT; case T_CLASS: case T_MODULE: if (FL_TEST_RAW(obj, RCLASS_BOXABLE)) { @@ -1232,7 +1232,7 @@ rb_shape_expected_layout(VALUE obj) } return SHAPE_ID_LAYOUT_RCLASS; case T_DATA: - return SHAPE_ID_LAYOUT_RDATA; + return SHAPE_ID_LAYOUT_EXTENDED; case T_IMEMO: if (IMEMO_TYPE_P(obj, imemo_fields)) { return SHAPE_ID_LAYOUT_ROBJECT; @@ -1243,6 +1243,23 @@ rb_shape_expected_layout(VALUE obj) } } +static const char * +shape_layout_name(shape_id_t shape_id) +{ + switch (rb_shape_layout(shape_id)) { + case SHAPE_ID_LAYOUT_ROBJECT: + return "robject"; + case SHAPE_ID_LAYOUT_RCLASS: + return "rclass"; + case SHAPE_ID_LAYOUT_EXTENDED: + return "extended (or RData)"; + case SHAPE_ID_LAYOUT_OTHER: + return "other"; + default: + return "invalid"; + } +} + bool rb_shape_verify_capacity_consistency_p(VALUE obj) { @@ -1268,8 +1285,8 @@ rb_shape_verify_consistency(VALUE obj, shape_id_t shape_id) shape_id_t actual_layout = rb_shape_layout(rb_obj_shape_id(obj)); shape_id_t expected_layout = rb_shape_expected_layout(obj); if (actual_layout != expected_layout) { - rb_bug("shape_id layout mismatch: expected=%x actual=%x shape_id=%u obj=%s", - expected_layout, actual_layout, shape_id, rb_obj_info(obj)); + rb_bug("shape_id layout mismatch: expected=%s actual=%s shape_id=%u obj=%s", + shape_layout_name(expected_layout), shape_layout_name(actual_layout), shape_id, rb_obj_info(obj)); } if (shape_id == ROOT_SHAPE_ID) { @@ -1373,8 +1390,8 @@ shape_layout(VALUE self) return ID2SYM(rb_intern("robject")); case SHAPE_ID_LAYOUT_RCLASS: return ID2SYM(rb_intern("rclass")); - case SHAPE_ID_LAYOUT_RDATA: - return ID2SYM(rb_intern("rdata")); + case SHAPE_ID_LAYOUT_EXTENDED: + return ID2SYM(rb_intern("extended_or_rdata")); case SHAPE_ID_LAYOUT_OTHER: return ID2SYM(rb_intern("other")); default: diff --git a/shape.h b/shape.h index 86f98b670782fb..51ebf4f72d762f 100644 --- a/shape.h +++ b/shape.h @@ -35,6 +35,8 @@ STATIC_ASSERT(shape_id_num_bits, SHAPE_ID_NUM_BITS == sizeof(shape_id_t) * CHAR_ // 29-30 SHAPE_ID_LAYOUT_MASK // The object's physical field layout. +STATIC_ASSERT(robject_rdata_fields_offset, offsetof(struct RObject, as.extended) == offsetof(struct RTypedData, fields_obj)); + enum shape_id_fl_type { #define RBIMPL_SHAPE_ID_FL(n) (1<<(SHAPE_ID_FL_USHIFT+n)) @@ -52,19 +54,25 @@ enum shape_id_fl_type { // are found in the fields_obj found on the rclass struct SHAPE_ID_LAYOUT_RCLASS = RBIMPL_SHAPE_ID_FL(3), - // Means this object is an RData or RTypedData and IVs are found in the - // fields_obj found on the RData/RTypedData struct - SHAPE_ID_LAYOUT_RDATA = RBIMPL_SHAPE_ID_FL(4), + // Means this object is an extened RObject or a RTypedData and IVs are found in the + // fields_obj found on the RObject/RTypedData struct at offset `sizeof(VALUE) * 2`. + SHAPE_ID_LAYOUT_EXTENDED = RBIMPL_SHAPE_ID_FL(4), + SHAPE_ID_LAYOUT_RDATA = SHAPE_ID_LAYOUT_EXTENDED, // Means this is a complicated object: boxable classes, structs, objects // that store IVs on the geniv table - SHAPE_ID_LAYOUT_OTHER = SHAPE_ID_LAYOUT_RCLASS | SHAPE_ID_LAYOUT_RDATA, + SHAPE_ID_LAYOUT_OTHER = SHAPE_ID_LAYOUT_RCLASS | SHAPE_ID_LAYOUT_EXTENDED, SHAPE_ID_LAYOUT_MASK = SHAPE_ID_LAYOUT_OTHER, SHAPE_ID_FL_NON_CANONICAL_MASK = SHAPE_ID_FL_FROZEN | SHAPE_ID_FL_HAS_OBJECT_ID, SHAPE_ID_FLAGS_MASK = SHAPE_ID_CAPACITY_MASK | SHAPE_ID_FL_NON_CANONICAL_MASK | SHAPE_ID_FL_COMPLEX | SHAPE_ID_LAYOUT_MASK, + // These parts of the shape id are specific to the object. + // Typically, when replicating a shape transition from an object to + // its IMEMO/fields, these bits should be stripped. + // All other bits are shared between an IMEMO/fields and its owner. + SHAPE_ID_FL_PRIVATE_MASK = SHAPE_ID_LAYOUT_MASK|SHAPE_ID_CAPACITY_MASK, #undef RBIMPL_SHAPE_ID_FL }; @@ -201,16 +209,27 @@ RBASIC_SET_FULL_SHAPE_ID(VALUE obj, shape_id_t shape_id) RUBY_ASSERT(rb_shape_verify_consistency(obj, shape_id)); } +static inline shape_id_t rb_shape_transition_layout(shape_id_t, shape_id_t); + +static inline void +RBASIC_SET_SHAPE_ID_WITH_LAYOUT(VALUE obj, shape_id_t target_shape_id, shape_id_t layout) +{ + RUBY_ASSERT((layout & SHAPE_ID_LAYOUT_MASK) == layout); + shape_id_t current_shape_id = RBASIC_SHAPE_ID(obj); + current_shape_id = rb_shape_transition_layout(current_shape_id, layout); + current_shape_id = (current_shape_id & SHAPE_ID_FL_PRIVATE_MASK) | (target_shape_id & ~SHAPE_ID_FL_PRIVATE_MASK); + RBASIC_SET_FULL_SHAPE_ID(obj, current_shape_id); +} + static inline void RBASIC_SET_SHAPE_ID(VALUE obj, shape_id_t shape_id) { RUBY_ASSERT(!RB_SPECIAL_CONST_P(obj)); - shape_id = ( - (shape_id & ~(SHAPE_ID_CAPACITY_MASK|SHAPE_ID_LAYOUT_MASK)) | - (RBASIC_SHAPE_ID(obj) & (SHAPE_ID_CAPACITY_MASK|SHAPE_ID_LAYOUT_MASK)) - ); - RBASIC_SET_FULL_SHAPE_ID(obj, shape_id); + RBASIC_SET_FULL_SHAPE_ID(obj, ( + (shape_id & ~SHAPE_ID_FL_PRIVATE_MASK) | + (RBASIC_SHAPE_ID(obj) & SHAPE_ID_FL_PRIVATE_MASK) + )); } static inline shape_id_t @@ -380,17 +399,7 @@ ROBJECT_FIELDS_HASH(VALUE obj) RUBY_ASSERT(rb_obj_shape_complex_p(obj)); RUBY_ASSERT(FL_TEST_RAW(obj, ROBJECT_HEAP)); - return ROBJECT(obj)->as.hash; -} - -static inline void -ROBJECT_SET_FIELDS_HASH(VALUE obj, st_table *tbl) -{ - RBIMPL_ASSERT_TYPE(obj, RUBY_T_OBJECT); - RUBY_ASSERT(rb_obj_shape_complex_p(obj)); - RUBY_ASSERT(FL_TEST_RAW(obj, ROBJECT_HEAP)); - - ROBJECT(obj)->as.hash = tbl; + return rb_imemo_fields_complex_tbl(ROBJECT(obj)->as.extended); } static inline uint32_t @@ -539,6 +548,12 @@ rb_shape_transition_slot_size(shape_id_t shape_id, size_t slot_size) return rb_shape_transition_capacity(shape_id, rb_shape_capacity_for_slot_size(slot_size)); } +static inline shape_id_t +rb_shape_transition_layout(shape_id_t shape_id, shape_id_t layout) +{ + return (shape_id & (~SHAPE_ID_LAYOUT_MASK)) | layout; +} + shape_id_t rb_shape_transition_object_id(shape_id_t shape_id); static inline shape_id_t diff --git a/spec/bundler/commands/exec_spec.rb b/spec/bundler/commands/exec_spec.rb index d744fc616bf238..e7a294b42932fa 100644 --- a/spec/bundler/commands/exec_spec.rb +++ b/spec/bundler/commands/exec_spec.rb @@ -75,13 +75,13 @@ skip "https://github.com/ruby/rubygems/issues/3351" if mswin? install_gemfile "source \"https://gem.repo1\"; gem \"myrack\"" bundle "exec #{gem_cmd} --version" - expect(out).to eq(Gem::VERSION) + expect(out).to eq(exercised_rubygems_version.to_s) end it "works when exec'ing to rubygems through sh -c" do install_gemfile "source \"https://gem.repo1\"; gem \"myrack\"" bundle "exec sh -c '#{gem_cmd} --version'" - expect(out).to eq(Gem::VERSION) + expect(out).to eq(exercised_rubygems_version.to_s) end it "works when exec'ing back to bundler to run a remote resolve" do diff --git a/spec/bundler/commands/install_spec.rb b/spec/bundler/commands/install_spec.rb index f8a134f231089e..1e0e3a7c244b42 100644 --- a/spec/bundler/commands/install_spec.rb +++ b/spec/bundler/commands/install_spec.rb @@ -1394,7 +1394,7 @@ def run expect(gem_make_out).not_to include("make -j8") end - it "uses 3 slots from the available pool when running the compilation of an extension" do + it "uses 3 slots from the available pool when running the compilation of an extension", rubygems: ">= 4.1.0.dev" do ENV.delete("MAKEFLAGS") install_gemfile(<<~G, env: { "BUNDLE_JOBS" => "8" }) @@ -1407,7 +1407,7 @@ def run expect(gem_make_out).to include("make -j3") end - it "consumes 3 slots from the pool when BUNDLE_JOBS isn't set" do + it "consumes 3 slots from the pool when BUNDLE_JOBS isn't set", rubygems: ">= 4.1.0.dev" do ENV.delete("MAKEFLAGS") install_gemfile(<<~G) diff --git a/spec/bundler/install/gemfile/git_spec.rb b/spec/bundler/install/gemfile/git_spec.rb index ee4c91a4f2b4a6..5fa740f4e1c68b 100644 --- a/spec/bundler/install/gemfile/git_spec.rb +++ b/spec/bundler/install/gemfile/git_spec.rb @@ -114,9 +114,12 @@ sha = git.ref_for("main", 11) spec_file = default_bundle_path("bundler/gems/foo-1.0-#{sha}/foo.gemspec") expect(spec_file).to exist - ruby_code = Gem::Specification.load(spec_file.to_s).to_ruby + # Serialize with the RubyGems that wrote the file, since `#to_ruby` + # output differs across RubyGems versions + ruby "print Gem::Specification.load(#{spec_file.to_s.dump}).to_ruby" + ruby_code = out file_code = File.read(spec_file) - expect(file_code).to eq(ruby_code) + expect(file_code.strip).to eq(ruby_code) end it "does not update the git source implicitly" do diff --git a/spec/bundler/install/gems/resolving_spec.rb b/spec/bundler/install/gems/resolving_spec.rb index 111d361aab9c4f..178f68e30bb676 100644 --- a/spec/bundler/install/gems/resolving_spec.rb +++ b/spec/bundler/install/gems/resolving_spec.rb @@ -726,7 +726,7 @@ Because every version of require_rubygems depends on RubyGems > 9000 and Gemfile depends on require_rubygems >= 0, RubyGems > 9000 is required. - So, because current RubyGems version is = #{Gem::VERSION}, + So, because current RubyGems version is = #{exercised_rubygems_version}, version solving has failed. E expect(err).to end_with(nice_error) diff --git a/spec/bundler/install/global_cache_spec.rb b/spec/bundler/install/global_cache_spec.rb index 259e8d57ba2ebc..5e5494dbe9d349 100644 --- a/spec/bundler/install/global_cache_spec.rb +++ b/spec/bundler/install/global_cache_spec.rb @@ -9,9 +9,10 @@ let(:source2) { "http://gemserver.example.org" } def cache_base - # Use the unified global gem cache path if available (from RubyGems), - # otherwise fall back to the Bundler-specific cache location - if Gem.respond_to?(:global_gem_cache_path) + # Use the unified global gem cache path if the RubyGems under test + # provides it, otherwise fall back to the Bundler-specific cache + # location that Bundler uses on RubyGems older than 4.0 + if exercised_rubygems_version >= Gem::Version.new("4.0.0.a") Pathname.new(Gem.global_gem_cache_path) else home(".bundle", "cache", "gems") diff --git a/spec/bundler/runtime/self_management_spec.rb b/spec/bundler/runtime/self_management_spec.rb index 176c2a3121774d..970a3869367fd0 100644 --- a/spec/bundler/runtime/self_management_spec.rb +++ b/spec/bundler/runtime/self_management_spec.rb @@ -171,11 +171,9 @@ expect(out).to eq(previous_minor) end - it "requires the right bundler version from the config and run bundle CLI without re-exec" do - unless Bundler.rubygems.provides?(">= 4.1.0.dev") - skip "This spec can only run when Gem::BundlerVersionFinder.bundler_versions reads bundler configs" - end - + # Can only run when Gem::BundlerVersionFinder.bundler_versions reads + # bundler configs + it "requires the right bundler version from the config and run bundle CLI without re-exec", rubygems: ">= 4.1.0.dev" do lockfile_bundled_with(current_version) bundle_config "version #{previous_minor}" diff --git a/spec/bundler/support/filters.rb b/spec/bundler/support/filters.rb index 2be25b4a78a6fe..67a32354d081b9 100644 --- a/spec/bundler/support/filters.rb +++ b/spec/bundler/support/filters.rb @@ -23,7 +23,10 @@ def inspect RSpec.configure do |config| config.filter_run_excluding realworld: true - config.filter_run_excluding rubygems: RequirementChecker.against(Gem.rubygems_version) + # Version-gated specs care about the RubyGems that spec-spawned processes + # run, which under RGV=system is older than the one loaded in this process. + exercised_rubygems_version = Gem::Version.new(ENV["BUNDLER_SPEC_SYSTEM_RUBYGEMS_VERSION"] || Gem::VERSION) + config.filter_run_excluding rubygems: RequirementChecker.against(exercised_rubygems_version) config.filter_run_excluding git: RequirementChecker.against(git_version) config.filter_run_excluding ruby_repo: !ENV["GEM_COMMAND"].nil? config.filter_run_excluding no_color_tty: Gem.win_platform? || !ENV["GITHUB_ACTION"].nil? diff --git a/spec/bundler/support/helpers.rb b/spec/bundler/support/helpers.rb index b0d4b5008ba742..8798cca57bb546 100644 --- a/spec/bundler/support/helpers.rb +++ b/spec/bundler/support/helpers.rb @@ -183,9 +183,22 @@ def gembin(cmd, options = {}) sys_exec(cmd.to_s, options) end + # The RubyGems version that processes spawned by specs run. Under + # RGV=system it's the system RubyGems, which is older than the one loaded + # in the current process. + def exercised_rubygems_version + @exercised_rubygems_version ||= Gem::Version.new(ENV["BUNDLER_SPEC_SYSTEM_RUBYGEMS_VERSION"] || Gem::VERSION) + end + def sys_exec(cmd, options = {}, &block) env = options[:env] || {} - env["RUBYOPT"] = opt_add(opt_add("-r#{spec_dir}/support/switch_rubygems.rb", env["RUBYOPT"]), ENV["RUBYOPT"]) + if ENV["RGV"] == "system" + # Spawned processes must boot system RubyGems untouched, so don't make + # them switch, and drop the load path override the harness runs with. + env["RUBYOPT"] = opt_add(env["RUBYOPT"] || "", opt_remove("-I#{source_lib_dir}", ENV["RUBYOPT"])) + else + env["RUBYOPT"] = opt_add(opt_add("-r#{spec_dir}/support/switch_rubygems.rb", env["RUBYOPT"]), ENV["RUBYOPT"]) + end options[:env] = env sh(cmd, options, &block) diff --git a/spec/bundler/support/switch_rubygems.rb b/spec/bundler/support/switch_rubygems.rb index 640b9f83b7d116..0958a1e827753c 100644 --- a/spec/bundler/support/switch_rubygems.rb +++ b/spec/bundler/support/switch_rubygems.rb @@ -2,4 +2,17 @@ require_relative "rubygems_version_manager" ENV["RGV"] ||= "." -RubygemsVersionManager.new(ENV["RGV"]).switch + +# RGV=system runs specs against system RubyGems: processes spawned by specs +# boot it untouched (see `Spec::Helpers#sys_exec`). The test harness itself +# still runs on the checked out RubyGems, because the repo's lib/ dir is +# unavoidably on its load path and would get mixed into an older RubyGems. +# The system RubyGems version is captured here, while it's still the one +# loaded, so version-gated specs can check the version actually under test. +source = ENV["RGV"] +if source == "system" + ENV["BUNDLER_SPEC_SYSTEM_RUBYGEMS_VERSION"] ||= Gem::VERSION + source = "." +end + +RubygemsVersionManager.new(source).switch diff --git a/spec/ruby/library/objectspace/memsize_of_spec.rb b/spec/ruby/library/objectspace/memsize_of_spec.rb index 9c8aea4e84ea0a..a99d69d17032f3 100644 --- a/spec/ruby/library/objectspace/memsize_of_spec.rb +++ b/spec/ruby/library/objectspace/memsize_of_spec.rb @@ -23,12 +23,20 @@ end it "is larger if the Object has more instance variables" do - obj = Object.new - before = ObjectSpace.memsize_of(obj) - 100.times do |i| - obj.instance_variable_set(:"@foo#{i}", nil) + before = ObjectSpace.memsize_of(Object.new) + + klass = Class.new do + set_ivar = 100.times.map { |i| "@foo#{i} = nil" } + class_eval(<<~RUBY, __FILE__, __LINE__ + 1) + def initialize + #{set_ivar.join("; ")} + end + RUBY end - after = ObjectSpace.memsize_of(obj) + + klass.new # in case the runtime needs warmup + + after = ObjectSpace.memsize_of(klass.new) after.should > before end end diff --git a/test/objspace/test_objspace.rb b/test/objspace/test_objspace.rb index 84cd88659ee2f0..403e98e6e1608d 100644 --- a/test/objspace/test_objspace.rb +++ b/test/objspace/test_objspace.rb @@ -895,6 +895,11 @@ def test_name_error_message bar rescue => err _, m = ObjectSpace.reachable_objects_from(err) + + # The very first NameError allocated by a process is extended + if ObjectSpace::InternalObjectWrapper === m # T_IMEMO/fields_obj + m, _ = ObjectSpace.reachable_objects_from(m) + end end assert_equal(m, m.clone) end diff --git a/test/ruby/test_shapes.rb b/test/ruby/test_shapes.rb index cbf4eebfa37a8a..8dc52eee54bf04 100644 --- a/test/ruby/test_shapes.rb +++ b/test/ruby/test_shapes.rb @@ -1106,8 +1106,8 @@ def test_shape_layout klass.instance_variable_set(:@a, 123) assert_equal :rclass, RubyVM::Shape.of(klass).layout - assert_equal :rdata, RubyVM::Shape.of(Thread.current).layout - assert_equal :rdata, RubyVM::Shape.of(lambda {}).layout + assert_equal :extended_or_rdata, RubyVM::Shape.of(Thread.current).layout + assert_equal :extended_or_rdata, RubyVM::Shape.of(lambda {}).layout assert_equal :other, RubyVM::Shape.of(Struct.new(:x).new(1)).layout assert_equal :other, RubyVM::Shape.of([]).layout diff --git a/thread_pthread.c b/thread_pthread.c index e0190093287976..d6e27950570942 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -1664,16 +1664,13 @@ rb_ractor_sched_barrier_join(rb_vm_t *vm, rb_ractor_t *cr) ractor_sched_lock(vm, cr); { // running_cnt - /* A not-yet-running thread (blocking/terminating, joined only to - * sync) must not be counted, or barrier_waiting_cnt > running_cnt-1. */ - if (cr->threads.sched.is_running) { - vm->ractor.sched.barrier_waiting_cnt++; - RUBY_DEBUG_LOG("waiting_cnt:%u serial:%u", vm->ractor.sched.barrier_waiting_cnt, barrier_serial); - ractor_sched_barrier_join_signal_locked(vm); - } - else { - RUBY_DEBUG_LOG("join without counting (not running) serial:%u", barrier_serial); - } + /* Every joiner is a member of the running set: a dying thread now + * leaves the living set before handing over its scheduler slot. */ + VM_ASSERT(ractor_sched_running_threads_contain_p(vm, GET_THREAD())); + vm->ractor.sched.barrier_waiting_cnt++; + RUBY_DEBUG_LOG("waiting_cnt:%u serial:%u", vm->ractor.sched.barrier_waiting_cnt, barrier_serial); + + ractor_sched_barrier_join_signal_locked(vm); ractor_sched_barrier_join_wait_locked(vm, cr->threads.sched.running); } ractor_sched_unlock(vm, cr); @@ -1747,6 +1744,12 @@ thread_sched_atfork(struct rb_thread_sched *sched) ccan_list_head_init(&vm->ractor.sched.grq); vm->ractor.sched.grq_cnt = 0; // the list was just emptied; reset the count with it + // A fork during a VM barrier leaves the child with barrier state that can + // never complete (the other ractors are gone); reset it like the rest. + vm->ractor.sched.barrier_waiting = false; + vm->ractor.sched.barrier_waiting_cnt = 0; + vm->ractor.sched.barrier_ractor = NULL; + vm->ractor.sched.barrier_lock_rec = 0; // Threads that were winding down in the parent do not exist in the child; // without this reset the child's ruby_vm_destruct would wait for their // reclaim (which never comes) forever. @@ -2500,6 +2503,7 @@ thread_sched_reclaim(struct coroutine_context *dead_co) SIZED_FREE(tctx); // pairs with the increment at the top of coroutine_thread_terminated: // a waiting VM destruct may proceed once this reclaim is done + VM_ASSERT(RUBY_ATOMIC_LOAD(GET_VM()->ractor.sched.winding_cnt) > 0); RUBY_ATOMIC_DEC(GET_VM()->ractor.sched.winding_cnt); return true; } diff --git a/thread_pthread_mn.c b/thread_pthread_mn.c index 422aae7b00034e..2f94aa248b0743 100644 --- a/thread_pthread_mn.c +++ b/thread_pthread_mn.c @@ -448,9 +448,8 @@ native_thread_check_and_create_shared(rb_vm_t *vm) } // A coroutine thread's epilogue, run from thread_start_func_2 while th is -// still valid (and, until the living-set removal, still GC-reachable). -// Everything that touches th happens here; co_start then only makes the -// final transfer, touching the execution-owned tctx alone. +// still valid. Everything that touches th happens here; co_start then only +// makes the final transfer, touching the execution-owned tctx alone. static void coroutine_thread_terminated(rb_thread_t *th) { @@ -470,6 +469,21 @@ coroutine_thread_terminated(rb_thread_t *th) rb_thread_t *wake_th; + // Leave the living set BEFORE handing over the scheduler slot: the + // removal's VM-lock work (ractor_check_blocking, a barrier join) then + // runs as an ordinary counted running thread. Afterwards th may be + // unreachable, but no GC can complete while th still owns the slot + // (a barrier waits for it to join), so the handoff below may keep + // touching th/sched. + // + // The Ractor's last thread is the exception and keeps the reverse + // order (below): its removal unlinks the Ractor itself, after which + // r/sched must not be touched. That order is safe only for it: with + // no successor, sched->running stays NULL, so the removal's VM lock + // never joins a barrier (vm_need_barrier requires a running thread). + VM_ASSERT(sched->running == th); // th owns the slot through the removal + if (!last) rb_ractor_living_threads_remove(r, th); + thread_sched_lock(sched, th); { // designate the successor (running = next from readyq, or NULL); for @@ -477,12 +491,12 @@ coroutine_thread_terminated(rb_thread_t *th) thread_sched_to_dead_common(sched, th); // Only the successor WE designated here may be enqueued by this - // epilogue (below, after the living-set removal). If readyq was - // empty, running is now NULL and a waker (e.g. the timer thread) - // that later installs a runnable thread enqueues the Ractor itself - // -- enqueuing "whatever is running" at that point would duplicate - // its entry. While running is non-NULL, nobody else re-assigns it, - // so wake_th stays valid until we enqueue. + // epilogue (below). If readyq was empty, running is now NULL and a + // waker (e.g. the timer thread) that later installs a runnable + // thread enqueues the Ractor itself -- enqueuing "whatever is + // running" at that point would duplicate its entry. While running + // is non-NULL, nobody else re-assigns it, so wake_th stays valid + // until we enqueue. wake_th = is_dnt ? NULL : sched->running; tctx->nt = th->nt; // stash the final transfer target for co_start @@ -491,14 +505,14 @@ coroutine_thread_terminated(rb_thread_t *th) } thread_sched_unlock(sched, th); - // Leave the living set. This is the dying thread's last access to th -- - // the removal needs the current ec (VM lock), and after it the GC may - // collect th (and, for a Ractor's main thread, the Ractor). - // (interrupt_lock stays valid up to here -- a concurrent terminate_all - // may interrupt any still-listed thread; it is destroyed in thread_free.) - rb_ractor_living_threads_remove(r, th); - if (last) { + // The reverse order is safe only with no successor: running == NULL + // means the removal's VM lock cannot join a barrier (vm_need_barrier). + VM_ASSERT(sched->running == NULL); + VM_ASSERT(wake_th == NULL); + // Last access to th/r: the removal may unlink the Ractor, after + // which the GC may collect th and r. + rb_ractor_living_threads_remove(r, th); rb_current_ec_set(NULL); // TLS only; r may be collectable already } else { diff --git a/variable.c b/variable.c index 994feb7a202d8b..bcd8c32ea2246b 100644 --- a/variable.c +++ b/variable.c @@ -1274,6 +1274,22 @@ rb_obj_fields_generic_uncached(VALUE obj) return fields_obj; } +static bool +obj_use_generic_fields_tbl_p(VALUE obj) +{ + switch (BUILTIN_TYPE(obj)) { + case T_OBJECT: + case T_CLASS: + case T_MODULE: + case T_DATA: + return false; + case T_STRUCT: + return !FL_TEST_RAW(obj, RSTRUCT_GEN_FIELDS); + default: + return true; + } +} + VALUE rb_obj_fields(VALUE obj, ID field_name) { @@ -1283,6 +1299,9 @@ rb_obj_fields(VALUE obj, ID field_name) VALUE fields_obj = 0; if (rb_obj_shape_has_fields(obj)) { switch (BUILTIN_TYPE(obj)) { + case T_OBJECT: + fields_obj = ROBJECT_FIELDS_OBJ(obj); + break; case T_DATA: fields_obj = RTYPEDDATA(obj)->fields_obj; break; @@ -1360,10 +1379,15 @@ rb_obj_set_fields(VALUE obj, VALUE fields_obj, ID field_name, VALUE original_fie } RUBY_ASSERT(IMEMO_TYPE_P(fields_obj, imemo_fields)); - RUBY_ASSERT(!original_fields_obj || IMEMO_TYPE_P(original_fields_obj, imemo_fields)); + RUBY_ASSERT(!original_fields_obj || IMEMO_TYPE_P(original_fields_obj, imemo_fields) || RB_TYPE_P(original_fields_obj, T_OBJECT)); + int type = BUILTIN_TYPE(obj); if (fields_obj != original_fields_obj) { - switch (BUILTIN_TYPE(obj)) { + switch (type) { + case T_OBJECT: + RUBY_ASSERT(obj != fields_obj); + ROBJECT_SET_EXTENDED(obj, fields_obj); + break; case T_DATA: RB_OBJ_WRITE(obj, &RTYPEDDATA(obj)->fields_obj, fields_obj); break; @@ -1389,19 +1413,32 @@ rb_obj_set_fields(VALUE obj, VALUE fields_obj, ID field_name, VALUE original_fie } } - if (original_fields_obj) { + if (original_fields_obj && original_fields_obj != obj) { // Clear root shape to avoid triggering cleanup such as free_object_id. rb_imemo_fields_clear(original_fields_obj); } } - RBASIC_SET_SHAPE_ID(obj, RBASIC_SHAPE_ID(fields_obj)); + if (type == T_OBJECT) { + RBASIC_SET_SHAPE_ID_WITH_LAYOUT(obj, RBASIC_SHAPE_ID(fields_obj), SHAPE_ID_LAYOUT_EXTENDED); + } + else { + RBASIC_SET_SHAPE_ID(obj, RBASIC_SHAPE_ID(fields_obj)); + } } void rb_obj_replace_fields(VALUE obj, VALUE fields_obj) { - RB_VM_LOCKING() { + if (obj_use_generic_fields_tbl_p(obj)) { + // We'll first lookup the generic fields table and then insert + // into it, so lock once for both operations. + RB_VM_LOCKING() { + VALUE original_fields_obj = rb_obj_fields_no_ractor_check(obj); + rb_obj_set_fields(obj, fields_obj, 0, original_fields_obj); + } + } + else { VALUE original_fields_obj = rb_obj_fields_no_ractor_check(obj); rb_obj_set_fields(obj, fields_obj, 0, original_fields_obj); } @@ -1421,7 +1458,7 @@ rb_obj_field_get(VALUE obj, shape_id_t target_shape_id) fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj); break; case T_OBJECT: - fields_obj = obj; + fields_obj = ROBJECT_FIELDS_OBJ(obj); break; case T_IMEMO: RUBY_ASSERT(IMEMO_TYPE_P(obj, imemo_fields)); @@ -1472,7 +1509,7 @@ rb_ivar_lookup(VALUE obj, ID id, VALUE undef) fields_obj = obj; break; case T_OBJECT: - fields_obj = obj; + fields_obj = ROBJECT_FIELDS_OBJ(obj); break; default: fields_obj = rb_obj_fields(obj, id); @@ -1573,38 +1610,17 @@ static shape_id_t obj_transition_complex(VALUE obj, st_table *table) { RUBY_ASSERT(!rb_obj_shape_complex_p(obj)); + RUBY_ASSERT(!RB_TYPE_P(obj, T_IMEMO)); + shape_id_t shape_id = rb_obj_shape_transition_complex(obj); + VALUE fields_obj = rb_imemo_fields_new_complex_tbl(obj, shape_id, table, RB_OBJ_SHAREABLE_P(obj)); - switch (BUILTIN_TYPE(obj)) { - case T_OBJECT: - { - VALUE *old_fields = NULL; - uint32_t old_fields_len = 0; - if (FL_TEST_RAW(obj, ROBJECT_HEAP)) { - old_fields = ROBJECT_FIELDS(obj); - old_fields_len = ROBJECT_FIELDS_CAPACITY(obj); - } - else { - FL_SET_RAW(obj, ROBJECT_HEAP); - } - RBASIC_SET_SHAPE_ID(obj, shape_id); - ROBJECT_SET_FIELDS_HASH(obj, table); - if (old_fields) { - SIZED_FREE_N(old_fields, old_fields_len); - } - } - break; - case T_CLASS: - case T_MODULE: - case T_IMEMO: - UNREACHABLE; - break; - default: - { - VALUE fields_obj = rb_imemo_fields_new_complex_tbl(obj, shape_id, table, RB_OBJ_SHAREABLE_P(obj)); - rb_obj_replace_fields(obj, fields_obj); - } - } + rb_obj_set_fields(obj, fields_obj, 0, 0); + RBASIC_SET_SHAPE_ID(obj, shape_id); + + RUBY_ASSERT(FL_TEST_RAW(fields_obj, ROBJECT_HEAP)); + RUBY_ASSERT(rb_obj_shape_complex_p(obj)); + RUBY_ASSERT(rb_obj_shape_complex_p(fields_obj)); return shape_id; } @@ -1658,7 +1674,7 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef) } break; case T_OBJECT: - fields_obj = obj; + fields_obj = ROBJECT_FIELDS_OBJ(obj); break; default: { fields_obj = rb_obj_fields(obj, id); @@ -1681,12 +1697,7 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef) if (UNLIKELY(rb_shape_complex_p(next_shape_id))) { if (UNLIKELY(!rb_shape_complex_p(old_shape_id))) { - if (type == T_OBJECT) { - rb_evict_fields_to_hash(obj); - } - else { - fields_obj = imemo_fields_complex_from_obj(obj, fields_obj, next_shape_id); - } + fields_obj = imemo_fields_complex_from_obj(obj, fields_obj, next_shape_id); } st_data_t key = id; if (!st_delete(rb_imemo_fields_complex_tbl(fields_obj), &key, (st_data_t *)&val)) { @@ -1712,18 +1723,17 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef) MEMMOVE(&fields[removed_index], &fields[removed_index + 1], VALUE, trailing_fields); RBASIC_SET_SHAPE_ID(fields_obj, next_shape_id); - if (FL_TEST_RAW(fields_obj, OBJ_FIELD_HEAP)) { - if (rb_obj_embedded_size(new_fields_count) <= rb_gc_obj_slot_size(fields_obj)) { - // Re-embed objects when instances become small enough - // This is necessary because YJIT assumes that objects with the same shape - // have the same embeddedness for efficiency (avoid extra checks) - FL_UNSET_RAW(fields_obj, ROBJECT_HEAP); - MEMCPY(rb_imemo_fields_ptr(fields_obj), fields, VALUE, new_fields_count); - SIZED_FREE_N(fields, RSHAPE_CAPACITY(old_shape_id)); - } - else if (RSHAPE_CAPACITY(old_shape_id) != RSHAPE_CAPACITY(next_shape_id)) { - IMEMO_OBJ_FIELDS(fields_obj)->as.external.ptr = ruby_xrealloc_sized(fields, RSHAPE_CAPACITY(next_shape_id) * sizeof(VALUE), RSHAPE_CAPACITY(old_shape_id) * sizeof(VALUE)); + if (type == T_OBJECT && obj != fields_obj && new_fields_count == rb_shape_embedded_capacity(RBASIC_SHAPE_ID(obj))) { + // Re-embed objects when instances become small enough + // This is necessary because YJIT assumes that objects with the same shape + // have the same embeddedness for efficiency (avoid extra checks) + // Note: shapes have changed significantly since, we could not do this anymore. + VALUE *embedded_fields = ROBJECT_EMBEDDED_FIELDS(obj); + MEMCPY(embedded_fields, fields, VALUE, new_fields_count); + for (attr_index_t i = 0; i < new_fields_count; i++) { + RB_OBJ_WRITTEN(obj, Qundef, embedded_fields[i]); } + fields_obj = 0; } } else { @@ -1732,10 +1742,15 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef) } } - RBASIC_SET_SHAPE_ID(obj, next_shape_id); if (fields_obj != original_fields_obj) { switch (type) { case T_OBJECT: + if (!fields_obj) { + FL_UNSET_RAW(obj, ROBJECT_HEAP); + } + else if (fields_obj != obj) { + ROBJECT_SET_EXTENDED(obj, fields_obj); + } break; case T_CLASS: case T_MODULE: @@ -1747,6 +1762,18 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef) } } + if (type == T_OBJECT) { + if (!fields_obj || fields_obj == obj) { + RBASIC_SET_SHAPE_ID_WITH_LAYOUT(obj, next_shape_id, SHAPE_ID_LAYOUT_ROBJECT); + } + else { + RBASIC_SET_SHAPE_ID_WITH_LAYOUT(obj, next_shape_id, SHAPE_ID_LAYOUT_EXTENDED); + } + } + else { + RBASIC_SET_SHAPE_ID(obj, next_shape_id); + } + return val; } @@ -1764,9 +1791,9 @@ rb_obj_init_complex(VALUE obj, st_table *table) RUBY_ASSERT(RSHAPE_LEN(RBASIC_SHAPE_ID(obj)) == 0); if (rb_obj_shape_complex_p(obj)) { - st_table *old_table = ROBJECT_FIELDS_HASH(obj); - ROBJECT_SET_FIELDS_HASH(obj, table); - if (old_table) st_free_table(old_table); + shape_id_t shape_id = rb_obj_shape_transition_complex(obj); + VALUE fields_obj = rb_imemo_fields_new_complex_tbl(obj, shape_id, table, false); + ROBJECT_SET_EXTENDED(obj, fields_obj); } else { obj_transition_complex(obj, table); @@ -1847,8 +1874,7 @@ imemo_fields_set(VALUE owner, VALUE fields_obj, shape_id_t target_shape_id, ID f } else { attr_index_t index = RSHAPE_INDEX(target_shape_id); - - if (concurrent || index >= RSHAPE_CAPACITY(current_shape_id)) { + if (concurrent || index >= rb_shape_embedded_capacity(current_shape_id)) { return imemo_fields_copy_append(owner, original_fields_obj, current_shape_id, target_shape_id, val); } @@ -1904,23 +1930,6 @@ generic_ivar_set(VALUE obj, ID id, VALUE val) return generic_field_set(obj, target_shape_id, id, val); } -void -rb_ensure_iv_list_size(VALUE obj, uint32_t current_len, uint32_t new_capacity) -{ - RUBY_ASSERT(!rb_obj_shape_complex_p(obj)); - - if (FL_TEST_RAW(obj, ROBJECT_HEAP)) { - SIZED_REALLOC_N(ROBJECT(obj)->as.heap.fields, VALUE, new_capacity, current_len); - } - else { - VALUE *ptr = ROBJECT_FIELDS(obj); - VALUE *newptr = ALLOC_N(VALUE, new_capacity); - MEMCPY(newptr, ptr, VALUE, current_len); - FL_SET_RAW(obj, ROBJECT_HEAP); - ROBJECT(obj)->as.heap.fields = newptr; - } -} - static int rb_obj_copy_ivs_to_hash_table_i(ID key, VALUE val, st_data_t arg) { @@ -1952,31 +1961,54 @@ obj_field_set(VALUE obj, shape_id_t target_shape_id, ID field_name, VALUE val) current_shape_id = rb_evict_fields_to_hash(obj); } - if (RSHAPE_LEN(target_shape_id) > RSHAPE_LEN(current_shape_id)) { - RBASIC_SET_SHAPE_ID(obj, target_shape_id); - } + // may be T_OBJECT or imemo_fields + VALUE fields_obj = ROBJECT_FIELDS_OBJ(obj); + + RUBY_ASSERT(rb_obj_shape_complex_p(obj)); + RUBY_ASSERT(rb_obj_shape_complex_p(fields_obj)); if (!field_name) { field_name = RSHAPE_EDGE_NAME(target_shape_id); RUBY_ASSERT(field_name); } - st_insert(ROBJECT_FIELDS_HASH(obj), (st_data_t)field_name, (st_data_t)val); - RB_OBJ_WRITTEN(obj, Qundef, val); + st_insert(rb_imemo_fields_complex_tbl(fields_obj), (st_data_t)field_name, (st_data_t)val); + RB_OBJ_WRITTEN(fields_obj, Qundef, val); + + RBASIC_SET_SHAPE_ID(obj, target_shape_id); + if (obj != fields_obj) { + RBASIC_SET_SHAPE_ID(fields_obj, target_shape_id); + } return ATTR_INDEX_NOT_SET; } else { attr_index_t index = RSHAPE_INDEX(target_shape_id); - if (index >= RSHAPE_LEN(current_shape_id)) { - if (UNLIKELY(index >= RSHAPE_CAPACITY(current_shape_id))) { - rb_ensure_iv_list_size(obj, RSHAPE_CAPACITY(current_shape_id), RSHAPE_CAPACITY(target_shape_id)); - } + // may be T_OBJECT or imemo_fields + VALUE fields_obj = ROBJECT_FIELDS_OBJ(obj); + + if (index < RSHAPE_LEN(current_shape_id)) { + // Replace existing value; + RB_OBJ_WRITE(fields_obj, &rb_imemo_fields_ptr(fields_obj)[index], val); + return index; + } + + RUBY_ASSERT(index == RSHAPE_LEN(current_shape_id)); + + if (UNLIKELY(index >= RSHAPE_CAPACITY(current_shape_id))) { + fields_obj = imemo_fields_copy_append(obj, fields_obj, current_shape_id, target_shape_id, val); + ROBJECT_SET_EXTENDED(obj, fields_obj); + RBASIC_SET_FULL_SHAPE_ID(obj, rb_shape_transition_layout(target_shape_id, SHAPE_ID_LAYOUT_EXTENDED)); + } + else { + RB_OBJ_WRITE(fields_obj, &rb_imemo_fields_ptr(fields_obj)[index], val); RBASIC_SET_SHAPE_ID(obj, target_shape_id); } - RB_OBJ_WRITE(obj, &ROBJECT_FIELDS(obj)[index], val); + if (obj != fields_obj) { + RBASIC_SET_SHAPE_ID(fields_obj, target_shape_id); + } return index; } diff --git a/vm_eval.c b/vm_eval.c index 84bb40ba8034ac..049131b0914d7b 100644 --- a/vm_eval.c +++ b/vm_eval.c @@ -1080,6 +1080,7 @@ VALUE rb_funcallv(VALUE recv, ID mid, int argc, const VALUE *argv) { VM_ASSERT(ruby_thread_has_gvl_p()); + VM_ASSERT(!RB_TYPE_P(recv, T_IMEMO)); return rb_funcallv_scope(recv, mid, argc, argv, CALL_FCALL); } diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 06ad97691523a0..1abf7dd122aefa 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -1216,7 +1216,7 @@ vm_getivar(VALUE obj, ID id, const rb_iseq_t *iseq, IVC ic, const struct rb_call switch (BUILTIN_TYPE(obj)) { case T_OBJECT: - fields_obj = obj; + fields_obj = ROBJECT_FIELDS_OBJ(obj); break; case T_CLASS: case T_MODULE: @@ -1463,9 +1463,13 @@ vm_setivar(VALUE obj, VALUE val, rb_setivar_cache cache) break; } - RB_OBJ_WRITE(obj, &ROBJECT_FIELDS(obj)[cache.index], val); + VALUE fields_obj = ROBJECT_FIELDS_OBJ(obj); + RB_OBJ_WRITE(fields_obj, &rb_imemo_fields_ptr(fields_obj)[cache.index], val); if (shape_id != dest_shape_id) { RBASIC_SET_SHAPE_ID(obj, dest_shape_id); + if (fields_obj != obj) { + RBASIC_SET_SHAPE_ID(fields_obj, dest_shape_id); + } } RB_DEBUG_COUNTER_INC(ivar_set_ic_hit); diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 1f8b320c4d6749..0a9a0a796eb7a5 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -3015,20 +3015,20 @@ fn gen_get_ivar( } Some(ivar_index) => { let ivar_opnd = if receiver_t_object { - if comptime_receiver.embedded_p() { - // See ROBJECT_FIELDS() from include/ruby/internal/core/robject.h + let offs = ROBJECT_OFFSET_AS_ARY as i32 + (ivar_index * SIZEOF_VALUE) as i32; + // See ROBJECT_FIELDS() from include/ruby/internal/core/robject.h + if comptime_receiver.embedded_p() { // Load the variable - let offs = ROBJECT_OFFSET_AS_ARY as i32 + (ivar_index * SIZEOF_VALUE) as i32; Opnd::mem(64, recv, offs) - } else { + } else { // Compile time value is *not* embedded. - // Get a pointer to the extended table + // Get the T_IMEMO/fields let tbl_opnd = asm.load(Opnd::mem(64, recv, ROBJECT_OFFSET_AS_HEAP_FIELDS as i32)); - // Read the ivar from the extended table - Opnd::mem(64, tbl_opnd, (SIZEOF_VALUE * ivar_index) as i32) + // Read the ivar from the T_IMEMO/fields + Opnd::mem(64, tbl_opnd, offs) } } else { asm_comment!(asm, "call rb_ivar_get_at()"); @@ -3080,6 +3080,33 @@ fn gen_getinstancevariable( ) } +fn gen_trigger_wb( + asm: &mut Assembler, + recv: Opnd, + write_val: Opnd) +{ + asm.spill_regs(); // for ccall (unconditionally spill them for RegMappings consistency) + let skip_wb = asm.new_label("skip_wb"); + // If the value we're writing is an immediate, we don't need to WB + asm.test(write_val, (RUBY_IMMEDIATE_MASK as u64).into()); + asm.jnz(skip_wb); + + // If the value we're writing is nil or false, we don't need to WB + asm.cmp(write_val, Qnil.into()); + asm.jbe(skip_wb); + + asm_comment!(asm, "write barrier"); + asm.ccall( + rb_gc_writebarrier as *const u8, + vec![ + recv, + write_val, + ] + ); + + asm.write_label(skip_wb); +} + // Generate an IV write. // This function doesn't deal with writing the shape, or expanding an object // to use an IV buffer if necessary. That is the callers responsibility @@ -3089,30 +3116,44 @@ fn gen_write_iv( recv: Opnd, ivar_index: usize, set_value: Opnd, - extension_needed: bool) + extension_needed: bool, + skip_wb: bool) { // Compile time self is embedded and the ivar index lands within the object let embed_test_result = comptime_receiver.embedded_p() && !extension_needed; + let offs = ROBJECT_OFFSET_AS_ARY as i32 + (ivar_index * SIZEOF_VALUE) as i32; + if embed_test_result { // Find the IV offset - let offs = ROBJECT_OFFSET_AS_ARY as i32 + (ivar_index * SIZEOF_VALUE) as i32; let ivar_opnd = Opnd::mem(64, recv, offs); // Write the IV asm_comment!(asm, "write IV"); asm.mov(ivar_opnd, set_value); + + // If we know the stack value is an immediate, there's no need to + // generate WB code. + if !skip_wb { + gen_trigger_wb(asm, recv, set_value); + } } else { // Compile time value is *not* embedded. - // Get a pointer to the extended table + // Get a pointer to the extended T_IMEMO/fields let tbl_opnd = asm.load(Opnd::mem(64, recv, ROBJECT_OFFSET_AS_HEAP_FIELDS as i32)); // Write the ivar in to the extended table - let ivar_opnd = Opnd::mem(64, tbl_opnd, (SIZEOF_VALUE * ivar_index) as i32); + let ivar_opnd = Opnd::mem(64, tbl_opnd, offs); asm_comment!(asm, "write IV"); asm.mov(ivar_opnd, set_value); + + // If we know the stack value is an immediate, there's no need to + // generate WB code. + if !skip_wb { + gen_trigger_wb(asm, tbl_opnd, set_value); + } } } @@ -3183,7 +3224,7 @@ fn gen_set_ivar( // The current shape doesn't contain this iv, we need to transition to another shape. let mut new_shape_complex = false; - let new_shape = if !shape_complex && receiver_t_object && ivar_index.is_none() { + if !shape_complex && receiver_t_object && ivar_index.is_none() { let current_shape_id = comptime_receiver.shape_id_of(); // We don't need to check about imemo_fields here because we're definitely looking at a T_OBJECT. let klass = unsafe { rb_obj_class(comptime_receiver) }; @@ -3218,7 +3259,7 @@ fn gen_set_ivar( // If the receiver isn't a T_OBJECT, then just write out the IV write as a function call. // too-complex shapes can't use index access, so we use rb_ivar_get for them too. - if !receiver_t_object || shape_complex || new_shape_complex || megamorphic { + if !receiver_t_object || shape_complex || new_shape_complex || megamorphic || ivar_index.is_none() { // The function could raise FrozenError. // Note that this modifies REG_SP, which is why we do it first jit_prepare_non_leaf_call(jit, asm); @@ -3252,7 +3293,7 @@ fn gen_set_ivar( } } else { // Get the receiver - let mut recv = asm.load(if let StackOpnd(index) = recv_opnd { + let recv = asm.load(if let StackOpnd(index) = recv_opnd { asm.stack_opnd(index as i32) } else { Opnd::mem(64, CFP, RUBY_OFFSET_CFP_SELF) @@ -3278,44 +3319,8 @@ fn gen_set_ivar( let write_val; match ivar_index { - // If we don't have an instance variable index, then we need to - // transition out of the current shape. None => { - let (new_shape_id, needs_extension, ivar_index) = new_shape.unwrap(); - if let Some((current_capacity, new_capacity)) = needs_extension { - // Generate the C call so that runtime code will increase - // the capacity and set the buffer. - asm_comment!(asm, "call rb_ensure_iv_list_size"); - - // It allocates so can trigger GC, which takes the VM lock - // so could yield to a different ractor. - jit_prepare_call_with_gc(jit, asm); - asm.ccall(rb_ensure_iv_list_size as *const u8, - vec![ - recv, - Opnd::UImm(current_capacity.into()), - Opnd::UImm(new_capacity.into()) - ] - ); - - // Load the receiver again after the function call - recv = asm.load(if let StackOpnd(index) = recv_opnd { - asm.stack_opnd(index as i32) - } else { - Opnd::mem(64, CFP, RUBY_OFFSET_CFP_SELF) - }); - } - - write_val = asm.stack_opnd(0); - gen_write_iv(asm, comptime_receiver, recv, ivar_index, write_val, needs_extension.is_some()); - - asm_comment!(asm, "write shape"); - - let shape_id_offset = unsafe { rb_shape_id_offset() }; - let shape_opnd = Opnd::mem(SHAPE_ID_NUM_BITS as u8, recv, shape_id_offset); - - // Store the new shape - asm.store(shape_opnd, Opnd::UImm(new_shape_id as u64)); + panic!("ivar_index None should have been delegated to rb_vm_set_ivar_id") }, Some(ivar_index) => { @@ -3325,34 +3330,9 @@ fn gen_set_ivar( // made the transition already, then there's no reason to // update the shape on the object. Just set the IV. write_val = asm.stack_opnd(0); - gen_write_iv(asm, comptime_receiver, recv, ivar_index, write_val, false); + gen_write_iv(asm, comptime_receiver, recv, ivar_index, write_val, false, stack_type.is_imm()); }, } - - // If we know the stack value is an immediate, there's no need to - // generate WB code. - if !stack_type.is_imm() { - asm.spill_regs(); // for ccall (unconditionally spill them for RegMappings consistency) - let skip_wb = asm.new_label("skip_wb"); - // If the value we're writing is an immediate, we don't need to WB - asm.test(write_val, (RUBY_IMMEDIATE_MASK as u64).into()); - asm.jnz(skip_wb); - - // If the value we're writing is nil or false, we don't need to WB - asm.cmp(write_val, Qnil.into()); - asm.jbe(skip_wb); - - asm_comment!(asm, "write barrier"); - asm.ccall( - rb_gc_writebarrier as *const u8, - vec![ - recv, - write_val, - ] - ); - - asm.write_label(skip_wb); - } } let write_val = asm.stack_pop(1); // Keep write_val on stack during ccall for GC diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index 2a9646dfed21bf..47c62c4a7f3f63 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -1126,7 +1126,6 @@ extern "C" { pub fn rb_ivar_get_at_no_ractor_check(obj: VALUE, index: attr_index_t) -> VALUE; pub fn rb_gvar_get(arg1: ID) -> VALUE; pub fn rb_gvar_set(arg1: ID, arg2: VALUE) -> VALUE; - pub fn rb_ensure_iv_list_size(obj: VALUE, current_len: u32, newsize: u32); pub fn rb_vm_barrier(); pub fn rb_str_byte_substr(str_: VALUE, beg: VALUE, len: VALUE) -> VALUE; pub fn rb_str_substr_two_fixnums( diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index ad7d4556786799..9024971d1ac61d 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -288,7 +288,7 @@ impl ShapeId { match self.0 & SHAPE_ID_LAYOUT_MASK { SHAPE_ID_LAYOUT_ROBJECT => ShapeLayout::RObject, SHAPE_ID_LAYOUT_RCLASS => ShapeLayout::RClass, - SHAPE_ID_LAYOUT_RDATA => ShapeLayout::RData, + SHAPE_ID_LAYOUT_EXTENDED => ShapeLayout::RData, SHAPE_ID_LAYOUT_OTHER => ShapeLayout::Other, layout => unreachable!("unknown shape layout bits: {layout:#x}"), } diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index ff254f433ec2c2..b552e8323b3ec5 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -1513,11 +1513,13 @@ pub const SHAPE_ID_FL_FROZEN: shape_id_fl_type = 134217728; pub const SHAPE_ID_FL_HAS_OBJECT_ID: shape_id_fl_type = 268435456; pub const SHAPE_ID_LAYOUT_ROBJECT: shape_id_fl_type = 0; pub const SHAPE_ID_LAYOUT_RCLASS: shape_id_fl_type = 536870912; +pub const SHAPE_ID_LAYOUT_EXTENDED: shape_id_fl_type = 1073741824; pub const SHAPE_ID_LAYOUT_RDATA: shape_id_fl_type = 1073741824; pub const SHAPE_ID_LAYOUT_OTHER: shape_id_fl_type = 1610612736; pub const SHAPE_ID_LAYOUT_MASK: shape_id_fl_type = 1610612736; pub const SHAPE_ID_FL_NON_CANONICAL_MASK: shape_id_fl_type = 402653184; pub const SHAPE_ID_FLAGS_MASK: shape_id_fl_type = 2146959360; +pub const SHAPE_ID_FL_PRIVATE_MASK: shape_id_fl_type = 1677197312; pub type shape_id_fl_type = u32; pub const CONST_DEPRECATED: rb_const_flag_t = 256; pub const CONST_VISIBILITY_MASK: rb_const_flag_t = 255; @@ -2192,7 +2194,6 @@ unsafe extern "C" { pub fn rb_ivar_get_at_no_ractor_check(obj: VALUE, index: attr_index_t) -> VALUE; pub fn rb_gvar_get(arg1: ID) -> VALUE; pub fn rb_gvar_set(arg1: ID, arg2: VALUE) -> VALUE; - pub fn rb_ensure_iv_list_size(obj: VALUE, current_len: u32, newsize: u32); pub fn rb_vm_barrier(); pub fn rb_str_byte_substr(str_: VALUE, beg: VALUE, len: VALUE) -> VALUE; pub fn rb_str_substr_two_fixnums( diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 7225fcdc3800bb..5fd2369537dd7e 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -5344,13 +5344,6 @@ impl Function { elidable: true }) } - fn load_ivar_heap(&mut self, block: BlockId, recv: InsnId, id: ID, ivar_index: attr_index_t) -> InsnId { - // See ROBJECT_FIELDS() from include/ruby/internal/core/robject.h - let ptr = self.load_field(block, recv, FieldName::as_heap, ROBJECT_OFFSET_AS_HEAP_FIELDS, types::CPtr); - let offset = SIZEOF_VALUE_I32 * ivar_index as i32; - self.load_field(block, ptr, id.into(), offset, types::BasicObject) - } - fn load_ivar_embedded(&mut self, block: BlockId, recv: InsnId, id: ID, ivar_index: attr_index_t) -> InsnId { // See ROBJECT_FIELDS() from include/ruby/internal/core/robject.h let offset = ROBJECT_OFFSET_AS_ARY @@ -5390,11 +5383,7 @@ impl Function { self.load_ivar_embedded(block, fields_obj, id, ivar_index) }, ShapeLayout::RObject => { - if recv_type.flags().is_embedded() { - self.load_ivar_embedded(block, self_val, id, ivar_index) - } else { - self.load_ivar_heap(block, self_val, id, ivar_index) - } + self.load_ivar_embedded(block, self_val, id, ivar_index) }, ShapeLayout::Other => { // Non-T_OBJECT, non-class/module, non-typed-data: fall back to C call @@ -5444,10 +5433,20 @@ impl Function { // Instance variable writes on immediate values raise. return Err(Counter::setivar_fallback_immediate); } - if !profiled_type.flags().is_t_object() { - // Check if the receiver is a T_OBJECT - return Err(Counter::setivar_fallback_not_t_object); + + match profiled_type.shape().layout() { + ShapeLayout::RObject => { + // OK + } + ShapeLayout::RData => { + // FIXME: we side exit for now as we're missing SHAPE_ID_FL_PRIVATE_MASK handling. + return Err(Counter::setivar_fallback_not_t_object); + } + _ => { + return Err(Counter::setivar_fallback_not_t_object); + } } + assert!(profiled_type.shape().is_valid()); if profiled_type.shape().is_frozen() { // Can't set ivars on frozen objects @@ -5490,22 +5489,35 @@ impl Function { } fn emit_optimized_setivar(&mut self, block: BlockId, self_val: InsnId, id: ID, val: InsnId, spec: SetIvarSpec) { - // Current shape contains this ivar - let (ivar_storage, offset) = if spec.profiled_type.flags().is_embedded() { - // See ROBJECT_FIELDS() from include/ruby/internal/core/robject.h - let offset = ROBJECT_OFFSET_AS_ARY + (SIZEOF_VALUE * spec.ivar_index.to_usize()) as i32; - (self_val, offset) - } else { - let as_heap = self.load_field(block, self_val, FieldName::as_heap, ROBJECT_OFFSET_AS_HEAP_FIELDS, types::CPtr); - let offset = SIZEOF_VALUE_I32 * spec.ivar_index as i32; - (as_heap, offset) + let offset = ROBJECT_OFFSET_AS_ARY + (SIZEOF_VALUE * spec.ivar_index.to_usize()) as i32; + + // See ROBJECT_FIELDS() from include/ruby/internal/core/robject.h + let (ivar_storage, embedded) = match spec.profiled_type.shape().layout() { + ShapeLayout::RObject => { // AKA embedded + (self_val, true) + }, + ShapeLayout::RData => { // AKA extended + let fields = self.load_field(block, self_val, FieldName::as_heap, ROBJECT_OFFSET_AS_HEAP_FIELDS, types::BasicObject); + (fields, false) + }, + ShapeLayout::Other | ShapeLayout::RClass => { + panic!("This is a T_OBJECT only path (for now)") + } }; + self.push_insn(block, Insn::StoreField { recv: ivar_storage, id: id.into(), offset, val, num_bits: types::BasicObject.num_bits() }); - self.push_insn(block, Insn::WriteBarrier { recv: self_val, val }); + self.push_insn(block, Insn::WriteBarrier { recv: ivar_storage, val }); if spec.next_shape != spec.profiled_type.shape() { // Write the new shape ID let shape_id = self.push_insn(block, Insn::Const { val: Const::CShape(spec.next_shape) }); let shape_id_offset = unsafe { rb_shape_id_offset() }; + + if !embedded { + // FIXME: We need to strip the SHAPE_ID_FL_PRIVATE_MASK from the shape for `ivar_storage`. + // see `RBASIC_SET_SHAPE_ID`. + // This path is currently dead code, see the FIXME in `prepare_optimized_setivar` + self.push_insn(block, Insn::StoreField { recv: ivar_storage, id: FieldName::shape_id, offset: shape_id_offset, val: shape_id, num_bits: types::CShape.num_bits() }); + } self.push_insn(block, Insn::StoreField { recv: self_val, id: FieldName::shape_id, offset: shape_id_offset, val: shape_id, num_bits: types::CShape.num_bits() }); } } diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 8023368039055f..b75bd87f178346 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -9492,8 +9492,8 @@ mod hir_opt_tests { Jump bb4(v17) bb6(): v19:CShape[0x1003] = GuardBitEquals v12, CShape(0x1003) recompile - v21:CPtr = LoadField v11, :as_heap@0x1004 - v22:BasicObject = LoadField v21, :@foo@0x1005 + v21:RubyValue = LoadField v11, :fields_obj@0x1004 + v22:BasicObject = LoadField v21, :@foo@0x1004 Jump bb4(v22) bb4(v13:BasicObject): v25:Fixnum[1] = Const Value(1) @@ -9555,12 +9555,12 @@ mod hir_opt_tests { v15:CBool = IsBitEqual v12, v14 CondBranch v15, bb5(), bb6() bb5(): - v17:CPtr = LoadField v11, :as_heap@0x1002 - v18:BasicObject = LoadField v17, :@foo@0x1003 + v17:RubyValue = LoadField v11, :fields_obj@0x1002 + v18:BasicObject = LoadField v17, :@foo@0x1002 Jump bb4(v18) bb6(): - v20:CShape[0x1004] = GuardBitEquals v12, CShape(0x1004) recompile - v22:BasicObject = LoadField v11, :@foo@0x1005 + v20:CShape[0x1003] = GuardBitEquals v12, CShape(0x1003) recompile + v22:BasicObject = LoadField v11, :@foo@0x1004 Jump bb4(v22) bb4(v13:BasicObject): v25:Fixnum[1] = Const Value(1)