From 10621f7cb9a0c70e568f89cce47a02e878af6778 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 1 Jun 2023 14:55:36 -0700 Subject: Revert "Fix cvar caching when class is cloned" This reverts commit 77d1b082470790c17c24a2f406b4fec5d522636b. --- class.c | 3 --- ext/objspace/depend | 5 ----- gc.c | 30 ------------------------------ internal/class.h | 4 ---- test/ruby/test_variable.rb | 13 ------------- variable.c | 1 - vm_insnhelper.c | 20 ++++++++------------ yjit/src/cruby_bindings.inc.rs | 1 - 8 files changed, 8 insertions(+), 69 deletions(-) diff --git a/class.c b/class.c index 4ffb47c27a..114df2bbd7 100644 --- a/class.c +++ b/class.c @@ -446,12 +446,9 @@ cvc_table_copy(ID id, VALUE val, void *data) ent = ALLOC(struct rb_cvar_class_tbl_entry); ent->class_value = ctx->clone; - ent->cref = orig_entry->cref; ent->global_cvar_state = orig_entry->global_cvar_state; rb_id_table_insert(ctx->new_table, id, (VALUE)ent); - RB_OBJ_WRITTEN(ctx->clone, Qundef, ent->cref); - return ID_TABLE_CONTINUE; } diff --git a/ext/objspace/depend b/ext/objspace/depend index d1e8236eb8..91ae569d67 100644 --- a/ext/objspace/depend +++ b/ext/objspace/depend @@ -365,11 +365,6 @@ 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: $(hdrdir)/ruby/thread_native.h -objspace.o: $(top_srcdir)/ccan/check_type/check_type.h -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)/id_table.h objspace.o: $(top_srcdir)/internal.h objspace.o: $(top_srcdir)/internal/array.h diff --git a/gc.c b/gc.c index 707754a9e5..44336c0d71 100644 --- a/gc.c +++ b/gc.c @@ -7138,8 +7138,6 @@ gc_ref_update_from_offset(rb_objspace_t *objspace, VALUE obj) } } -static void mark_cvc_tbl(rb_objspace_t *objspace, VALUE klass); - static void gc_mark_children(rb_objspace_t *objspace, VALUE obj) { @@ -7190,7 +7188,6 @@ gc_mark_children(rb_objspace_t *objspace, VALUE obj) if (!RCLASS_EXT(obj)) break; mark_m_tbl(objspace, RCLASS_M_TBL(obj)); - mark_cvc_tbl(objspace, obj); cc_table_mark(objspace, obj); for (attr_index_t i = 0; i < RCLASS_IV_COUNT(obj); i++) { gc_mark(objspace, RCLASS_IVPTR(obj)[i]); @@ -10328,14 +10325,9 @@ static enum rb_id_table_iterator_result update_cvc_tbl_i(VALUE cvc_entry, void *data) { struct rb_cvar_class_tbl_entry *entry; - rb_objspace_t * objspace = (rb_objspace_t *)data; entry = (struct rb_cvar_class_tbl_entry *)cvc_entry; - if (entry->cref) { - TYPED_UPDATE_IF_MOVED(objspace, rb_cref_t *, entry->cref); - } - entry->class_value = rb_gc_location(entry->class_value); return ID_TABLE_CONTINUE; @@ -10350,28 +10342,6 @@ update_cvc_tbl(rb_objspace_t *objspace, VALUE klass) } } -static enum rb_id_table_iterator_result -mark_cvc_tbl_i(VALUE cvc_entry, void *data) -{ - struct rb_cvar_class_tbl_entry *entry; - - entry = (struct rb_cvar_class_tbl_entry *)cvc_entry; - - RUBY_ASSERT(entry->cref == 0 || (BUILTIN_TYPE((VALUE)entry->cref) == T_IMEMO && IMEMO_TYPE_P(entry->cref, imemo_cref))); - rb_gc_mark((VALUE) entry->cref); - - return ID_TABLE_CONTINUE; -} - -static void -mark_cvc_tbl(rb_objspace_t *objspace, VALUE klass) -{ - struct rb_id_table *tbl = RCLASS_CVC_TBL(klass); - if (tbl) { - rb_id_table_foreach_values(tbl, mark_cvc_tbl_i, objspace); - } -} - static enum rb_id_table_iterator_result update_const_table(VALUE value, void *data) { diff --git a/internal/class.h b/internal/class.h index d76da84bd1..b33c807e97 100644 --- a/internal/class.h +++ b/internal/class.h @@ -17,9 +17,6 @@ #include "ruby/intern.h" /* for rb_alloc_func_t */ #include "ruby/ruby.h" /* for struct RBasic */ #include "shape.h" -#include "ruby_assert.h" -#include "vm_core.h" -#include "method.h" /* for rb_cref_t */ #ifdef RCLASS_SUPER # undef RCLASS_SUPER @@ -35,7 +32,6 @@ typedef struct rb_subclass_entry rb_subclass_entry_t; struct rb_cvar_class_tbl_entry { uint32_t index; rb_serial_t global_cvar_state; - const rb_cref_t * cref; VALUE class_value; }; diff --git a/test/ruby/test_variable.rb b/test/ruby/test_variable.rb index e50b681ce8..1158313776 100644 --- a/test/ruby/test_variable.rb +++ b/test/ruby/test_variable.rb @@ -49,19 +49,6 @@ class TestVariable < Test::Unit::TestCase assert_equal(1, c.class_variable_get(:@@foo)) end - Zeus = Gods.clone - - def test_cloned_allows_setting_cvar - Zeus.class_variable_set(:@@rule, "Athena") - - god = Gods.new.ruler0 - zeus = Zeus.new.ruler0 - - assert_equal "Cronus", god - assert_equal "Athena", zeus - assert_not_equal god.object_id, zeus.object_id - end - def test_singleton_class_included_class_variable c = Class.new c.extend(Olympians) diff --git a/variable.c b/variable.c index 0491562d22..64cb07407c 100644 --- a/variable.c +++ b/variable.c @@ -3693,7 +3693,6 @@ rb_cvar_set(VALUE klass, ID id, VALUE val) ent = ALLOC(struct rb_cvar_class_tbl_entry); ent->class_value = target; ent->global_cvar_state = GET_GLOBAL_CVAR_STATE(); - ent->cref = 0; rb_id_table_insert(rb_cvc_tbl, id, (VALUE)ent); RB_DEBUG_COUNTER_INC(cvar_inline_miss); } diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 5ef1ad10de..9407a75981 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -1491,7 +1491,7 @@ vm_setivar(VALUE obj, ID id, VALUE val, shape_id_t dest_shape_id, attr_index_t i } static VALUE -update_classvariable_cache(const rb_iseq_t *iseq, VALUE klass, ID id, const rb_cref_t * cref, ICVARC ic) +update_classvariable_cache(const rb_iseq_t *iseq, VALUE klass, ID id, ICVARC ic) { VALUE defined_class = 0; VALUE cvar_value = rb_cvar_find(klass, id, &defined_class); @@ -1511,13 +1511,9 @@ update_classvariable_cache(const rb_iseq_t *iseq, VALUE klass, ID id, const rb_c } struct rb_cvar_class_tbl_entry *ent = (void *)ent_data; - ent->global_cvar_state = GET_GLOBAL_CVAR_STATE(); - ent->cref = cref; - ic->entry = ent; - RUBY_ASSERT(BUILTIN_TYPE((VALUE)cref) == T_IMEMO && IMEMO_TYPE_P(cref, imemo_cref)); - RB_OBJ_WRITTEN(iseq, Qundef, ent->cref); + ic->entry = ent; RB_OBJ_WRITTEN(iseq, Qundef, ent->class_value); return cvar_value; @@ -1527,9 +1523,8 @@ static inline VALUE vm_getclassvariable(const rb_iseq_t *iseq, const rb_control_frame_t *reg_cfp, ID id, ICVARC ic) { const rb_cref_t *cref; - cref = vm_get_cref(GET_EP()); - if (ic->entry && ic->entry->global_cvar_state == GET_GLOBAL_CVAR_STATE() && ic->entry->cref == cref && LIKELY(rb_ractor_main_p())) { + if (ic->entry && ic->entry->global_cvar_state == GET_GLOBAL_CVAR_STATE() && LIKELY(rb_ractor_main_p())) { RB_DEBUG_COUNTER_INC(cvar_read_inline_hit); VALUE v = rb_ivar_lookup(ic->entry->class_value, id, Qundef); @@ -1538,9 +1533,10 @@ vm_getclassvariable(const rb_iseq_t *iseq, const rb_control_frame_t *reg_cfp, ID return v; } + cref = vm_get_cref(GET_EP()); VALUE klass = vm_get_cvar_base(cref, reg_cfp, 1); - return update_classvariable_cache(iseq, klass, id, cref, ic); + return update_classvariable_cache(iseq, klass, id, ic); } VALUE @@ -1553,20 +1549,20 @@ static inline void vm_setclassvariable(const rb_iseq_t *iseq, const rb_control_frame_t *reg_cfp, ID id, VALUE val, ICVARC ic) { const rb_cref_t *cref; - cref = vm_get_cref(GET_EP()); - if (ic->entry && ic->entry->global_cvar_state == GET_GLOBAL_CVAR_STATE() && ic->entry->cref == cref && LIKELY(rb_ractor_main_p())) { + if (ic->entry && ic->entry->global_cvar_state == GET_GLOBAL_CVAR_STATE()) { RB_DEBUG_COUNTER_INC(cvar_write_inline_hit); rb_class_ivar_set(ic->entry->class_value, id, val); return; } + cref = vm_get_cref(GET_EP()); VALUE klass = vm_get_cvar_base(cref, reg_cfp, 1); rb_cvar_set(klass, id, val); - update_classvariable_cache(iseq, klass, id, cref, ic); + update_classvariable_cache(iseq, klass, id, ic); } void diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index 16bb6e1feb..048da7b0ff 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -806,7 +806,6 @@ pub type rb_shape_t = rb_shape; pub struct rb_cvar_class_tbl_entry { pub index: u32, pub global_cvar_state: rb_serial_t, - pub cref: *const rb_cref_t, pub class_value: VALUE, } pub const VM_CALL_ARGS_SPLAT_bit: vm_call_flag_bits = 0; -- cgit v1.2.3