From 6fce8c79807e69cfe475b5291e892567c869fbcc Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 25 Oct 2023 16:52:37 -0700 Subject: Don't try compacting ivars on Classes that are "too complex" Too complex classes use a hash table to store ivs, and should always pin their IVs. We shouldn't touch those classes in compaction. --- common.mk | 32 ++++++++++++++++++++++++++++++++ ext/objspace/depend | 5 +++++ gc.c | 6 ++++-- internal/class.h | 39 +++++++++++++++++++++++++++++++++++++++ shape.h | 8 -------- variable.c | 16 ---------------- yjit/src/cruby_bindings.inc.rs | 2 +- 7 files changed, 81 insertions(+), 27 deletions(-) diff --git a/common.mk b/common.mk index 4367fcbe1f..fa35c17124 100644 --- a/common.mk +++ b/common.mk @@ -3006,7 +3006,9 @@ class.$(OBJEXT): {$(VPATH)}subst.h class.$(OBJEXT): {$(VPATH)}thread_$(THREAD_MODEL).h class.$(OBJEXT): {$(VPATH)}thread_native.h class.$(OBJEXT): {$(VPATH)}vm_core.h +class.$(OBJEXT): {$(VPATH)}vm_debug.h class.$(OBJEXT): {$(VPATH)}vm_opts.h +class.$(OBJEXT): {$(VPATH)}vm_sync.h compar.$(OBJEXT): $(hdrdir)/ruby/ruby.h compar.$(OBJEXT): $(top_srcdir)/internal/basic_operators.h compar.$(OBJEXT): $(top_srcdir)/internal/compar.h @@ -3441,6 +3443,7 @@ compile.$(OBJEXT): {$(VPATH)}vm_callinfo.h compile.$(OBJEXT): {$(VPATH)}vm_core.h compile.$(OBJEXT): {$(VPATH)}vm_debug.h compile.$(OBJEXT): {$(VPATH)}vm_opts.h +compile.$(OBJEXT): {$(VPATH)}vm_sync.h compile.$(OBJEXT): {$(VPATH)}yjit.h complex.$(OBJEXT): $(CCAN_DIR)/check_type/check_type.h complex.$(OBJEXT): $(CCAN_DIR)/container_of/container_of.h @@ -3480,6 +3483,7 @@ complex.$(OBJEXT): {$(VPATH)}backward/2/stdarg.h complex.$(OBJEXT): {$(VPATH)}complex.c complex.$(OBJEXT): {$(VPATH)}config.h complex.$(OBJEXT): {$(VPATH)}constant.h +complex.$(OBJEXT): {$(VPATH)}debug_counter.h complex.$(OBJEXT): {$(VPATH)}defines.h complex.$(OBJEXT): {$(VPATH)}encoding.h complex.$(OBJEXT): {$(VPATH)}id.h @@ -3648,7 +3652,9 @@ complex.$(OBJEXT): {$(VPATH)}subst.h complex.$(OBJEXT): {$(VPATH)}thread_$(THREAD_MODEL).h complex.$(OBJEXT): {$(VPATH)}thread_native.h complex.$(OBJEXT): {$(VPATH)}vm_core.h +complex.$(OBJEXT): {$(VPATH)}vm_debug.h complex.$(OBJEXT): {$(VPATH)}vm_opts.h +complex.$(OBJEXT): {$(VPATH)}vm_sync.h cont.$(OBJEXT): $(CCAN_DIR)/check_type/check_type.h cont.$(OBJEXT): $(CCAN_DIR)/container_of/container_of.h cont.$(OBJEXT): $(CCAN_DIR)/list/list.h @@ -4075,6 +4081,7 @@ debug.$(OBJEXT): {$(VPATH)}vm_callinfo.h debug.$(OBJEXT): {$(VPATH)}vm_core.h debug.$(OBJEXT): {$(VPATH)}vm_debug.h debug.$(OBJEXT): {$(VPATH)}vm_opts.h +debug.$(OBJEXT): {$(VPATH)}vm_sync.h debug_counter.$(OBJEXT): $(hdrdir)/ruby/ruby.h debug_counter.$(OBJEXT): {$(VPATH)}assert.h debug_counter.$(OBJEXT): {$(VPATH)}backward/2/assume.h @@ -6175,6 +6182,7 @@ enumerator.$(OBJEXT): {$(VPATH)}backward/2/stdalign.h enumerator.$(OBJEXT): {$(VPATH)}backward/2/stdarg.h enumerator.$(OBJEXT): {$(VPATH)}config.h enumerator.$(OBJEXT): {$(VPATH)}constant.h +enumerator.$(OBJEXT): {$(VPATH)}debug_counter.h enumerator.$(OBJEXT): {$(VPATH)}defines.h enumerator.$(OBJEXT): {$(VPATH)}encoding.h enumerator.$(OBJEXT): {$(VPATH)}enumerator.c @@ -6344,7 +6352,9 @@ enumerator.$(OBJEXT): {$(VPATH)}subst.h enumerator.$(OBJEXT): {$(VPATH)}thread_$(THREAD_MODEL).h enumerator.$(OBJEXT): {$(VPATH)}thread_native.h enumerator.$(OBJEXT): {$(VPATH)}vm_core.h +enumerator.$(OBJEXT): {$(VPATH)}vm_debug.h enumerator.$(OBJEXT): {$(VPATH)}vm_opts.h +enumerator.$(OBJEXT): {$(VPATH)}vm_sync.h error.$(OBJEXT): $(CCAN_DIR)/check_type/check_type.h error.$(OBJEXT): $(CCAN_DIR)/container_of/container_of.h error.$(OBJEXT): $(CCAN_DIR)/list/list.h @@ -6783,6 +6793,7 @@ eval.$(OBJEXT): {$(VPATH)}vm.h eval.$(OBJEXT): {$(VPATH)}vm_core.h eval.$(OBJEXT): {$(VPATH)}vm_debug.h eval.$(OBJEXT): {$(VPATH)}vm_opts.h +eval.$(OBJEXT): {$(VPATH)}vm_sync.h explicit_bzero.$(OBJEXT): {$(VPATH)}config.h explicit_bzero.$(OBJEXT): {$(VPATH)}explicit_bzero.c explicit_bzero.$(OBJEXT): {$(VPATH)}internal/attr/format.h @@ -8489,7 +8500,9 @@ iseq.$(OBJEXT): {$(VPATH)}thread_native.h iseq.$(OBJEXT): {$(VPATH)}util.h iseq.$(OBJEXT): {$(VPATH)}vm_callinfo.h iseq.$(OBJEXT): {$(VPATH)}vm_core.h +iseq.$(OBJEXT): {$(VPATH)}vm_debug.h iseq.$(OBJEXT): {$(VPATH)}vm_opts.h +iseq.$(OBJEXT): {$(VPATH)}vm_sync.h iseq.$(OBJEXT): {$(VPATH)}yjit.h load.$(OBJEXT): $(CCAN_DIR)/check_type/check_type.h load.$(OBJEXT): $(CCAN_DIR)/container_of/container_of.h @@ -9235,6 +9248,7 @@ marshal.$(OBJEXT): {$(VPATH)}backward/2/stdarg.h marshal.$(OBJEXT): {$(VPATH)}builtin.h marshal.$(OBJEXT): {$(VPATH)}config.h marshal.$(OBJEXT): {$(VPATH)}constant.h +marshal.$(OBJEXT): {$(VPATH)}debug_counter.h marshal.$(OBJEXT): {$(VPATH)}defines.h marshal.$(OBJEXT): {$(VPATH)}encindex.h marshal.$(OBJEXT): {$(VPATH)}encoding.h @@ -9408,7 +9422,9 @@ marshal.$(OBJEXT): {$(VPATH)}thread_$(THREAD_MODEL).h marshal.$(OBJEXT): {$(VPATH)}thread_native.h marshal.$(OBJEXT): {$(VPATH)}util.h marshal.$(OBJEXT): {$(VPATH)}vm_core.h +marshal.$(OBJEXT): {$(VPATH)}vm_debug.h marshal.$(OBJEXT): {$(VPATH)}vm_opts.h +marshal.$(OBJEXT): {$(VPATH)}vm_sync.h math.$(OBJEXT): $(hdrdir)/ruby/ruby.h math.$(OBJEXT): $(top_srcdir)/internal/bignum.h math.$(OBJEXT): $(top_srcdir)/internal/class.h @@ -10664,6 +10680,7 @@ object.$(OBJEXT): {$(VPATH)}backward/2/stdarg.h object.$(OBJEXT): {$(VPATH)}builtin.h object.$(OBJEXT): {$(VPATH)}config.h object.$(OBJEXT): {$(VPATH)}constant.h +object.$(OBJEXT): {$(VPATH)}debug_counter.h object.$(OBJEXT): {$(VPATH)}defines.h object.$(OBJEXT): {$(VPATH)}encoding.h object.$(OBJEXT): {$(VPATH)}id.h @@ -10839,7 +10856,9 @@ object.$(OBJEXT): {$(VPATH)}thread_native.h object.$(OBJEXT): {$(VPATH)}util.h object.$(OBJEXT): {$(VPATH)}variable.h object.$(OBJEXT): {$(VPATH)}vm_core.h +object.$(OBJEXT): {$(VPATH)}vm_debug.h object.$(OBJEXT): {$(VPATH)}vm_opts.h +object.$(OBJEXT): {$(VPATH)}vm_sync.h pack.$(OBJEXT): $(CCAN_DIR)/check_type/check_type.h pack.$(OBJEXT): $(CCAN_DIR)/container_of/container_of.h pack.$(OBJEXT): $(CCAN_DIR)/list/list.h @@ -12440,6 +12459,7 @@ proc.$(OBJEXT): {$(VPATH)}backward/2/stdalign.h proc.$(OBJEXT): {$(VPATH)}backward/2/stdarg.h proc.$(OBJEXT): {$(VPATH)}config.h proc.$(OBJEXT): {$(VPATH)}constant.h +proc.$(OBJEXT): {$(VPATH)}debug_counter.h proc.$(OBJEXT): {$(VPATH)}defines.h proc.$(OBJEXT): {$(VPATH)}encoding.h proc.$(OBJEXT): {$(VPATH)}eval_intern.h @@ -12611,7 +12631,9 @@ proc.$(OBJEXT): {$(VPATH)}subst.h proc.$(OBJEXT): {$(VPATH)}thread_$(THREAD_MODEL).h proc.$(OBJEXT): {$(VPATH)}thread_native.h proc.$(OBJEXT): {$(VPATH)}vm_core.h +proc.$(OBJEXT): {$(VPATH)}vm_debug.h proc.$(OBJEXT): {$(VPATH)}vm_opts.h +proc.$(OBJEXT): {$(VPATH)}vm_sync.h proc.$(OBJEXT): {$(VPATH)}yjit.h process.$(OBJEXT): $(CCAN_DIR)/check_type/check_type.h process.$(OBJEXT): $(CCAN_DIR)/container_of/container_of.h @@ -15299,9 +15321,11 @@ rjit_c.$(OBJEXT): {$(VPATH)}thread_$(THREAD_MODEL).h rjit_c.$(OBJEXT): {$(VPATH)}thread_native.h rjit_c.$(OBJEXT): {$(VPATH)}vm_callinfo.h rjit_c.$(OBJEXT): {$(VPATH)}vm_core.h +rjit_c.$(OBJEXT): {$(VPATH)}vm_debug.h rjit_c.$(OBJEXT): {$(VPATH)}vm_exec.h rjit_c.$(OBJEXT): {$(VPATH)}vm_insnhelper.h rjit_c.$(OBJEXT): {$(VPATH)}vm_opts.h +rjit_c.$(OBJEXT): {$(VPATH)}vm_sync.h rjit_c.$(OBJEXT): {$(VPATH)}yjit.h ruby-runner.$(OBJEXT): {$(VPATH)}config.h ruby-runner.$(OBJEXT): {$(VPATH)}internal/compiler_is.h @@ -17168,6 +17192,7 @@ struct.$(OBJEXT): {$(VPATH)}backward/2/stdarg.h struct.$(OBJEXT): {$(VPATH)}builtin.h struct.$(OBJEXT): {$(VPATH)}config.h struct.$(OBJEXT): {$(VPATH)}constant.h +struct.$(OBJEXT): {$(VPATH)}debug_counter.h struct.$(OBJEXT): {$(VPATH)}defines.h struct.$(OBJEXT): {$(VPATH)}encoding.h struct.$(OBJEXT): {$(VPATH)}id.h @@ -17337,7 +17362,9 @@ struct.$(OBJEXT): {$(VPATH)}subst.h struct.$(OBJEXT): {$(VPATH)}thread_$(THREAD_MODEL).h struct.$(OBJEXT): {$(VPATH)}thread_native.h struct.$(OBJEXT): {$(VPATH)}vm_core.h +struct.$(OBJEXT): {$(VPATH)}vm_debug.h struct.$(OBJEXT): {$(VPATH)}vm_opts.h +struct.$(OBJEXT): {$(VPATH)}vm_sync.h symbol.$(OBJEXT): $(CCAN_DIR)/check_type/check_type.h symbol.$(OBJEXT): $(CCAN_DIR)/container_of/container_of.h symbol.$(OBJEXT): $(CCAN_DIR)/list/list.h @@ -19052,6 +19079,7 @@ vm_backtrace.$(OBJEXT): {$(VPATH)}backward/2/stdarg.h vm_backtrace.$(OBJEXT): {$(VPATH)}config.h vm_backtrace.$(OBJEXT): {$(VPATH)}constant.h vm_backtrace.$(OBJEXT): {$(VPATH)}debug.h +vm_backtrace.$(OBJEXT): {$(VPATH)}debug_counter.h vm_backtrace.$(OBJEXT): {$(VPATH)}defines.h vm_backtrace.$(OBJEXT): {$(VPATH)}encoding.h vm_backtrace.$(OBJEXT): {$(VPATH)}eval_intern.h @@ -19223,7 +19251,9 @@ vm_backtrace.$(OBJEXT): {$(VPATH)}thread_$(THREAD_MODEL).h vm_backtrace.$(OBJEXT): {$(VPATH)}thread_native.h vm_backtrace.$(OBJEXT): {$(VPATH)}vm_backtrace.c vm_backtrace.$(OBJEXT): {$(VPATH)}vm_core.h +vm_backtrace.$(OBJEXT): {$(VPATH)}vm_debug.h vm_backtrace.$(OBJEXT): {$(VPATH)}vm_opts.h +vm_backtrace.$(OBJEXT): {$(VPATH)}vm_sync.h vm_dump.$(OBJEXT): $(CCAN_DIR)/check_type/check_type.h vm_dump.$(OBJEXT): $(CCAN_DIR)/container_of/container_of.h vm_dump.$(OBJEXT): $(CCAN_DIR)/list/list.h @@ -19840,7 +19870,9 @@ vm_trace.$(OBJEXT): {$(VPATH)}thread_$(THREAD_MODEL).h vm_trace.$(OBJEXT): {$(VPATH)}thread_native.h vm_trace.$(OBJEXT): {$(VPATH)}trace_point.rbinc vm_trace.$(OBJEXT): {$(VPATH)}vm_core.h +vm_trace.$(OBJEXT): {$(VPATH)}vm_debug.h vm_trace.$(OBJEXT): {$(VPATH)}vm_opts.h +vm_trace.$(OBJEXT): {$(VPATH)}vm_sync.h vm_trace.$(OBJEXT): {$(VPATH)}vm_trace.c vm_trace.$(OBJEXT): {$(VPATH)}yjit.h weakmap.$(OBJEXT): $(CCAN_DIR)/check_type/check_type.h diff --git a/ext/objspace/depend b/ext/objspace/depend index 40c2f90a3c..85e99a71b4 100644 --- a/ext/objspace/depend +++ b/ext/objspace/depend @@ -378,6 +378,7 @@ objspace.o: $(top_srcdir)/ccan/container_of/container_of.h objspace.o: $(top_srcdir)/ccan/list/list.h objspace.o: $(top_srcdir)/ccan/str/str.h objspace.o: $(top_srcdir)/constant.h +objspace.o: $(top_srcdir)/debug_counter.h objspace.o: $(top_srcdir)/id_table.h objspace.o: $(top_srcdir)/internal.h objspace.o: $(top_srcdir)/internal/array.h @@ -402,7 +403,9 @@ objspace.o: $(top_srcdir)/shape.h objspace.o: $(top_srcdir)/symbol.h objspace.o: $(top_srcdir)/thread_pthread.h objspace.o: $(top_srcdir)/vm_core.h +objspace.o: $(top_srcdir)/vm_debug.h objspace.o: $(top_srcdir)/vm_opts.h +objspace.o: $(top_srcdir)/vm_sync.h objspace.o: objspace.c objspace.o: {$(VPATH)}id.h objspace_dump.o: $(RUBY_EXTCONF_H) @@ -613,7 +616,9 @@ objspace_dump.o: $(top_srcdir)/symbol.h objspace_dump.o: $(top_srcdir)/thread_pthread.h objspace_dump.o: $(top_srcdir)/vm_callinfo.h objspace_dump.o: $(top_srcdir)/vm_core.h +objspace_dump.o: $(top_srcdir)/vm_debug.h objspace_dump.o: $(top_srcdir)/vm_opts.h +objspace_dump.o: $(top_srcdir)/vm_sync.h objspace_dump.o: objspace.h objspace_dump.o: objspace_dump.c objspace_dump.o: {$(VPATH)}id.h diff --git a/gc.c b/gc.c index 35e231136f..0c3a73a9f0 100644 --- a/gc.c +++ b/gc.c @@ -10608,8 +10608,10 @@ gc_update_object_references(rb_objspace_t *objspace, VALUE obj) update_cvc_tbl(objspace, obj); update_superclasses(objspace, obj); - for (attr_index_t i = 0; i < RCLASS_IV_COUNT(obj); i++) { - UPDATE_IF_MOVED(objspace, RCLASS_IVPTR(obj)[i]); + if (!rb_shape_obj_too_complex(obj)) { + for (attr_index_t i = 0; i < RCLASS_IV_COUNT(obj); i++) { + UPDATE_IF_MOVED(objspace, RCLASS_IVPTR(obj)[i]); + } } update_class_ext(objspace, RCLASS_EXT(obj)); diff --git a/internal/class.h b/internal/class.h index b055f07317..324e7b3219 100644 --- a/internal/class.h +++ b/internal/class.h @@ -19,6 +19,7 @@ #include "shape.h" #include "ruby_assert.h" #include "vm_core.h" +#include "vm_sync.h" #include "method.h" /* for rb_cref_t */ #ifdef RCLASS_SUPER @@ -111,6 +112,44 @@ struct RClass_and_rb_classext_t { #define RCLASS_SUPERCLASSES_INCLUDE_SELF FL_USER2 #define RICLASS_ORIGIN_SHARED_MTBL FL_USER3 +static inline st_table * +RCLASS_IV_HASH(VALUE obj) +{ + RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE)); + RUBY_ASSERT(rb_shape_obj_too_complex(obj)); + return (st_table *)RCLASS_IVPTR(obj); +} + +static inline void +RCLASS_SET_IV_HASH(VALUE obj, const st_table *tbl) +{ + RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE)); + RUBY_ASSERT(rb_shape_obj_too_complex(obj)); + RCLASS_IVPTR(obj) = (VALUE *)tbl; +} + +static inline uint32_t +RCLASS_IV_COUNT(VALUE obj) +{ + RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE)); + if (rb_shape_obj_too_complex(obj)) { + uint32_t count; + + // "Too complex" classes could have their IV hash mutated in + // parallel, so lets lock around getting the hash size. + RB_VM_LOCK_ENTER(); + { + count = (uint32_t)rb_st_table_size(RCLASS_IV_HASH(obj)); + } + RB_VM_LOCK_LEAVE(); + + return count; + } + else { + return rb_shape_get_shape_by_id(RCLASS_SHAPE_ID(obj))->next_iv_index; + } +} + /* class.c */ void rb_class_subclass_add(VALUE super, VALUE klass); void rb_class_remove_from_super_subclasses(VALUE); diff --git a/shape.h b/shape.h index 5b8420a402..a222a75ad8 100644 --- a/shape.h +++ b/shape.h @@ -213,14 +213,6 @@ RBASIC_IV_COUNT(VALUE obj) return rb_shape_get_shape_by_id(rb_shape_get_shape_id(obj))->next_iv_index; } -static inline uint32_t -RCLASS_IV_COUNT(VALUE obj) -{ - RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE)); - uint32_t ivc = rb_shape_get_shape_by_id(RCLASS_SHAPE_ID(obj))->next_iv_index; - return ivc; -} - rb_shape_t *rb_shape_traverse_from_new_root(rb_shape_t *initial_shape, rb_shape_t *orig_shape); bool rb_shape_set_shape_id(VALUE obj, shape_id_t shape_id); diff --git a/variable.c b/variable.c index 511d5c7d54..b6c9cc3e0d 100644 --- a/variable.c +++ b/variable.c @@ -63,22 +63,6 @@ static void setup_const_entry(rb_const_entry_t *, VALUE, VALUE, rb_const_flag_t) static VALUE rb_const_search(VALUE klass, ID id, int exclude, int recurse, int visibility); static st_table *generic_iv_tbl_; -static inline st_table * -RCLASS_IV_HASH(VALUE obj) -{ - RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE)); - RUBY_ASSERT(rb_shape_obj_too_complex(obj)); - return (st_table *)RCLASS_IVPTR(obj); -} - -static inline void -RCLASS_SET_IV_HASH(VALUE obj, const st_table *tbl) -{ - RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE)); - RUBY_ASSERT(rb_shape_obj_too_complex(obj)); - RCLASS_IVPTR(obj) = (VALUE *)tbl; -} - void Init_var_tables(void) { diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index 431314ff86..5eb96e9993 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -983,6 +983,7 @@ extern "C" { 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, len: u32, newsize: u32); + pub fn rb_vm_barrier(); pub fn rb_obj_as_string_result(str_: VALUE, obj: VALUE) -> VALUE; pub fn rb_str_concat_literals(num: usize, strary: *const VALUE) -> VALUE; pub fn rb_ec_str_resurrect(ec: *mut rb_execution_context_struct, str_: VALUE) -> VALUE; @@ -1005,7 +1006,6 @@ extern "C" { pub fn rb_iseq_line_no(iseq: *const rb_iseq_t, pos: usize) -> ::std::os::raw::c_uint; pub fn rb_iseqw_to_iseq(iseqw: VALUE) -> *const rb_iseq_t; pub fn rb_iseq_label(iseq: *const rb_iseq_t) -> VALUE; - pub fn rb_vm_barrier(); pub fn rb_profile_frames( start: ::std::os::raw::c_int, limit: ::std::os::raw::c_int, -- cgit v1.2.3