[Feature #22135] Remove obsolete ObjectSpace._id2ref

Merged: https://github.com/ruby/ruby/pull/17577

NEWS.md

@@ -43,6 +43,10 @@ Note: We're only listing outstanding class updates.
     * `MatchData#integer_at` is added.  It converts the matched substring to
       integer and return the result.  [[Feature #21932]]
 
+* ObjectSpace
+
+    *  `ObjectSpace._id2ref` was removed.  [[Feature #22135]]
+
 * Regexp
 
     * All instances of `Regexp` are now frozen, not just literals.

bootstraptest/test_objectspace.rb

@@ -1,18 +1,3 @@
-assert_normal_exit %q{
-  eval("", TOPLEVEL_BINDING)
-  minobj = ObjectSpace.to_enum(:each_object).min_by {|a| a.object_id }
-  maxobj = ObjectSpace.to_enum(:each_object).max_by {|a| a.object_id }
-  (((minobj.object_id-100)..(minobj.object_id+100))+
-   ((maxobj.object_id-100)..(maxobj.object_id+100))).each {|id|
-    begin
-      o = ObjectSpace._id2ref(id)
-    rescue RangeError
-      next
-    end
-    o.inspect if defined?(o.inspect)
-  }
-}, '[ruby-dev:31911]'
-
 assert_normal_exit %q{
   ary = (1..10).to_a
   ary.permutation(2) {|x|
@@ -44,12 +29,3 @@ assert_normal_exit %q{
     Thread.new {}
   end
 }, '[ruby-core:37858]'
-
-assert_equal 'ok', %q{
-  objects_and_ids = 1000.times.map { o = Object.new; [o, o.object_id] }
-  objects_and_ids.each { |expected, id|
-    actual = ObjectSpace._id2ref(id)
-    raise "expected #{expected.inspect}, got #{actual.inspect}" unless actual.equal?(expected)
-  }
-  'ok'
-}

bootstraptest/test_ractor.rb

@@ -1256,37 +1256,6 @@ assert_equal '0', %q{
   }.value
 }
 
-# ObjectSpace._id2ref can not handle unshareable objects with Ractors
-assert_equal 'ok', <<~'RUBY', frozen_string_literal: false
-  s = 'hello'
-
-  Ractor.new s.object_id do |id ;s|
-    begin
-      s = ObjectSpace._id2ref(id)
-    rescue => e
-      :ok
-    end
-  end.value
-RUBY
-
-# Inserting into the id2ref table should be Ractor-safe
-assert_equal 'ok', <<~'RUBY'
-  # Force all calls to Kernel#object_id to insert into the id2ref table
-  obj = Object.new
-  ObjectSpace._id2ref(obj.object_id) rescue nil
-
-  10.times.map do
-    Ractor.new do
-      10_000.times do
-        a = Object.new
-        a.object_id
-      end
-    end
-  end.map(&:value)
-
-  :ok
-RUBY
-
 # Ractor.make_shareable(obj)
 assert_equal 'true', <<~'RUBY', frozen_string_literal: false
   class C
@@ -2175,19 +2144,6 @@ assert_equal 'ok', %q{
   roundtripped_obj.instance_variable_get(:@array1) == [1] ? :ok : roundtripped_obj
 }
 
-# move object with generic ivars and existing id2ref table
-# [Bug #21664]
-assert_equal 'ok', %q{
-  obj = [1]
-  obj.instance_variable_set("@field", :ok)
-  ObjectSpace._id2ref(obj.object_id) # build id2ref table
-
-  ractor = Ractor.new { Ractor.receive }
-  ractor.send(obj, move: true)
-  obj = ractor.value
-  obj.instance_variable_get("@field")
-}
-
 # copy object with complex generic ivars
 assert_equal 'ok', %q{
   # Make Array complex

gc.c

@@ -586,7 +586,7 @@ rb_gc_guarded_ptr_val(volatile VALUE *ptr, VALUE val)
 #endif
 
 static const char *obj_type_name(VALUE obj);
-static st_table *id2ref_tbl;
+
 #include "gc/default/default.c"
 
 #if USE_MODULAR_GC && !defined(HAVE_DLOPEN)
@@ -1353,7 +1353,7 @@ rb_gc_imemo_needs_cleanup_p(VALUE obj)
         return ((rb_imemo_tmpbuf_t *)obj)->ptr != NULL;
 
       case imemo_fields:
-        return FL_TEST_RAW(obj, OBJ_FIELD_HEAP) || (id2ref_tbl && rb_obj_shape_has_id(obj));
+        return FL_TEST_RAW(obj, OBJ_FIELD_HEAP);
     }
     UNREACHABLE_RETURN(true);
 }
@@ -1365,7 +1365,6 @@ rb_gc_imemo_needs_cleanup_p(VALUE obj)
  * Objects that return false are:
  * - Simple embedded objects without external allocations
  * - Objects without finalizers
- * - Objects without object IDs registered in id2ref
  * - Objects without generic instance variables
  *
  * This is used by the GC sweep fast path to avoid function call overhead
@@ -1405,7 +1404,6 @@ rb_gc_obj_needs_cleanup_p(VALUE obj)
     }
 
     shape_id_t shape_id = RBASIC_SHAPE_ID(obj);
-    if (id2ref_tbl && rb_shape_has_object_id(shape_id)) return true;
 
     switch (flags & RUBY_T_MASK) {
       case T_OBJECT:
@@ -2054,7 +2052,6 @@ rb_objspace_garbage_object_p(VALUE obj)
 
 #define OBJ_ID_INCREMENT (RUBY_IMMEDIATE_MASK + 1)
 #define LAST_OBJECT_ID() (object_id_counter * OBJ_ID_INCREMENT)
-static VALUE id2ref_value = 0;
 
 #if SIZEOF_SIZE_T == SIZEOF_LONG_LONG
 static size_t object_id_counter = 1;
@@ -2076,76 +2073,8 @@ generate_next_object_id(void)
 #endif
 }
 
-void
-rb_gc_obj_id_moved(VALUE obj)
-{
-    if (UNLIKELY(id2ref_tbl)) {
-        st_insert(id2ref_tbl, (st_data_t)rb_obj_id(obj), (st_data_t)obj);
-    }
-}
-
-static int
-object_id_cmp(st_data_t x, st_data_t y)
-{
-    if (RB_TYPE_P(x, T_BIGNUM)) {
-        return !rb_big_eql(x, y);
-    }
-    else {
-        return x != y;
-    }
-}
-
-static st_index_t
-object_id_hash(st_data_t n)
-{
-    return FIX2LONG(rb_hash((VALUE)n));
-}
-
-static const struct st_hash_type object_id_hash_type = {
-    object_id_cmp,
-    object_id_hash,
-};
-
 static void gc_mark_tbl_no_pin(st_table *table);
 
-static void
-id2ref_tbl_mark(void *data)
-{
-    st_table *table = (st_table *)data;
-    if (UNLIKELY(!RB_POSFIXABLE(LAST_OBJECT_ID()))) {
-        // It's very unlikely, but if enough object ids were generated, keys may be T_BIGNUM
-        rb_mark_set(table);
-    }
-    // We purposely don't mark values, as they are weak references.
-    // rb_gc_obj_free_vm_weak_references takes care of cleaning them up.
-}
-
-static size_t
-id2ref_tbl_memsize(const void *data)
-{
-    return rb_st_memsize(data);
-}
-
-static void
-id2ref_tbl_free(void *data)
-{
-    id2ref_tbl = NULL; // clear global ref
-    st_table *table = (st_table *)data;
-    st_free_table(table);
-}
-
-static const rb_data_type_t id2ref_tbl_type = {
-    .wrap_struct_name = "VM/_id2ref_table",
-    .function = {
-        .dmark = id2ref_tbl_mark,
-        .dfree = id2ref_tbl_free,
-        .dsize = id2ref_tbl_memsize,
-        // dcompact function not required because the table is reference updated
-        // in rb_gc_vm_weak_table_foreach
-    },
-    .flags = RUBY_TYPED_WB_PROTECTED | RUBY_TYPED_FREE_IMMEDIATELY
-};
-
 static VALUE
 class_object_id(VALUE klass)
 {
@@ -2157,9 +2086,6 @@ class_object_id(VALUE klass)
         if (existing_id) {
             id = existing_id;
         }
-        else if (RB_UNLIKELY(id2ref_tbl)) {
-            st_insert(id2ref_tbl, id, klass);
-        }
         RB_GC_VM_UNLOCK(lock_lev);
     }
     return id;
@@ -2203,11 +2129,6 @@ object_id0(VALUE obj)
 
     RUBY_ASSERT(rb_obj_shape_has_id(obj));
 
-    if (RB_UNLIKELY(id2ref_tbl)) {
-        RB_VM_LOCKING() {
-            st_insert(id2ref_tbl, (st_data_t)id, (st_data_t)obj);
-        }
-    }
     return id;
 }
 
@@ -2238,123 +2159,9 @@ object_id(VALUE obj)
     return object_id0(obj);
 }
 
-static void
-build_id2ref_i(VALUE obj, void *data)
-{
-    st_table *id2ref_tbl = (st_table *)data;
-
-    switch (BUILTIN_TYPE(obj)) {
-      case T_CLASS:
-      case T_MODULE:
-        RUBY_ASSERT(!rb_objspace_garbage_object_p(obj));
-        if (RCLASS(obj)->object_id) {
-            st_insert(id2ref_tbl, RCLASS(obj)->object_id, obj);
-        }
-        break;
-      case T_IMEMO:
-        RUBY_ASSERT(!rb_objspace_garbage_object_p(obj));
-        if (IMEMO_TYPE_P(obj, imemo_fields) && rb_obj_shape_has_id(obj)) {
-            st_insert(id2ref_tbl, rb_obj_id(obj), rb_imemo_fields_owner(obj));
-        }
-        break;
-      case T_OBJECT:
-        RUBY_ASSERT(!rb_objspace_garbage_object_p(obj));
-        if (rb_obj_shape_has_id(obj)) {
-            st_insert(id2ref_tbl, rb_obj_id(obj), obj);
-        }
-        break;
-      default:
-        // For generic_fields, the T_IMEMO/fields is responsible for populating the entry.
-        break;
-    }
-}
-
-static VALUE
-object_id_to_ref(void *objspace_ptr, VALUE object_id)
-{
-    rb_objspace_t *objspace = objspace_ptr;
-
-    unsigned int lev = RB_GC_VM_LOCK();
-
-    if (!id2ref_tbl) {
-        rb_gc_vm_barrier(); // stop other ractors
-
-        // GC Must not trigger while we build the table, otherwise if we end
-        // up freeing an object that had an ID, we might try to delete it from
-        // the table even though it wasn't inserted yet.
-        st_table *tmp_id2ref_tbl = st_init_table(&object_id_hash_type);
-        VALUE tmp_id2ref_value = TypedData_Wrap_Struct(0, &id2ref_tbl_type, tmp_id2ref_tbl);
-
-        // build_id2ref_i will most certainly malloc, which could trigger GC and sweep
-        // objects we just added to the table.
-        // By calling rb_gc_disable() we also save having to handle potentially garbage objects.
-        bool gc_disabled = RTEST(rb_gc_disable());
-        {
-            id2ref_tbl = tmp_id2ref_tbl;
-            id2ref_value = tmp_id2ref_value;
-
-            rb_gc_impl_each_object(objspace, build_id2ref_i, (void *)id2ref_tbl);
-        }
-        if (!gc_disabled) rb_gc_enable();
-    }
-
-    VALUE obj;
-    bool found = st_lookup(id2ref_tbl, object_id, &obj) && !rb_gc_impl_garbage_object_p(objspace, obj);
-
-    RB_GC_VM_UNLOCK(lev);
-
-    if (found) {
-        return obj;
-    }
-
-    if (rb_funcall(object_id, rb_intern(">="), 1, ULL2NUM(LAST_OBJECT_ID()))) {
-        rb_raise(rb_eRangeError, "%+"PRIsVALUE" is not an id value", rb_funcall(object_id, rb_intern("to_s"), 1, INT2FIX(10)));
-    }
-    else {
-        rb_raise(rb_eRangeError, "%+"PRIsVALUE" is a recycled object", rb_funcall(object_id, rb_intern("to_s"), 1, INT2FIX(10)));
-    }
-}
-
 static inline void
 obj_free_object_id(VALUE obj)
 {
-    VALUE obj_id = 0;
-    if (RB_UNLIKELY(id2ref_tbl)) {
-        switch (BUILTIN_TYPE(obj)) {
-          case T_CLASS:
-          case T_MODULE:
-            obj_id = RCLASS(obj)->object_id;
-            break;
-          case T_IMEMO:
-            if (!IMEMO_TYPE_P(obj, imemo_fields)) {
-                return;
-            }
-            // fallthrough
-          case T_OBJECT:
-            {
-            shape_id_t shape_id = RBASIC_SHAPE_ID(obj);
-            if (rb_shape_has_object_id(shape_id)) {
-                obj_id = object_id_get(obj, shape_id);
-            }
-            break;
-          }
-          default:
-            // For generic_fields, the T_IMEMO/fields is responsible for freeing the id.
-            return;
-        }
-
-        if (RB_UNLIKELY(obj_id)) {
-            RUBY_ASSERT(FIXNUM_P(obj_id) || RB_TYPE_P(obj_id, T_BIGNUM));
-
-            if (!st_delete(id2ref_tbl, (st_data_t *)&obj_id, NULL)) {
-                // The the object is a T_IMEMO/fields, then it's possible the actual object
-                // has been garbage collected already.
-                if (!RB_TYPE_P(obj, T_IMEMO)) {
-                    rb_bug("Object ID seen, but not in _id2ref table: object_id=%llu object=%s", NUM2ULL(obj_id), rb_obj_info(obj));
-                }
-            }
-        }
-    }
 }
 
 void
@@ -2393,67 +2200,6 @@ rb_gc_obj_free_vm_weak_references(VALUE obj)
     }
 }
 
-/*
- *  call-seq:
- *     ObjectSpace._id2ref(object_id) -> an_object
- *
- *  Converts an object id to a reference to the object. May not be
- *  called on an object id passed as a parameter to a finalizer.
- *
- *     s = "I am a string"                    #=> "I am a string"
- *     r = ObjectSpace._id2ref(s.object_id)   #=> "I am a string"
- *     r == s                                 #=> true
- *
- *  On multi-ractor mode, if the object is not shareable, it raises
- *  RangeError.
- *
- *  This method is deprecated and should no longer be used.
- */
-
-static VALUE
-id2ref(VALUE objid)
-{
-    objid = rb_to_int(objid);
-    if (FIXNUM_P(objid) || rb_big_size(objid) <= SIZEOF_VOIDP) {
-        VALUE ptr = (VALUE)NUM2PTR(objid);
-        if (SPECIAL_CONST_P(ptr)) {
-            if (ptr == Qtrue) return Qtrue;
-            if (ptr == Qfalse) return Qfalse;
-            if (NIL_P(ptr)) return Qnil;
-            if (FIXNUM_P(ptr)) return ptr;
-            if (FLONUM_P(ptr)) return ptr;
-
-            if (SYMBOL_P(ptr)) {
-                // Check that the symbol is valid
-                if (rb_static_id_valid_p(SYM2ID(ptr))) {
-                    return ptr;
-                }
-                else {
-                    rb_raise(rb_eRangeError, "%p is not a symbol id value", (void *)ptr);
-                }
-            }
-
-            rb_raise(rb_eRangeError, "%+"PRIsVALUE" is not an id value", rb_int2str(objid, 10));
-        }
-    }
-
-    VALUE obj = object_id_to_ref(rb_gc_get_objspace(), objid);
-    if (!rb_multi_ractor_p() || rb_ractor_shareable_p(obj)) {
-        return obj;
-    }
-    else {
-        rb_raise(rb_eRangeError, "%+"PRIsVALUE" is the id of an unshareable object on multi-ractor", rb_int2str(objid, 10));
-    }
-}
-
-/* :nodoc: */
-static VALUE
-os_id2ref(VALUE os, VALUE objid)
-{
-    rb_category_warn(RB_WARN_CATEGORY_DEPRECATED, "ObjectSpace._id2ref is deprecated");
-    return id2ref(objid);
-}
-
 static VALUE
 rb_find_object_id(void *objspace, VALUE obj, VALUE (*get_heap_object_id)(VALUE))
 {
@@ -4208,33 +3954,6 @@ vm_weak_table_sym_set_foreach(VALUE *sym_ptr, void *data)
 
 struct st_table *rb_generic_fields_tbl_get(void);
 
-static int
-vm_weak_table_id2ref_foreach(st_data_t key, st_data_t value, st_data_t data, int error)
-{
-    struct global_vm_table_foreach_data *iter_data = (struct global_vm_table_foreach_data *)data;
-
-    if (!iter_data->weak_only && !FIXNUM_P((VALUE)key)) {
-        int ret = iter_data->callback((VALUE)key, iter_data->data);
-        if (ret != ST_CONTINUE) return ret;
-    }
-
-    return iter_data->callback((VALUE)value, iter_data->data);
-}
-
-static int
-vm_weak_table_id2ref_foreach_update(st_data_t *key, st_data_t *value, st_data_t data, int existing)
-{
-    struct global_vm_table_foreach_data *iter_data = (struct global_vm_table_foreach_data *)data;
-
-    iter_data->update_callback((VALUE *)value, iter_data->data);
-
-    if (!iter_data->weak_only && !FIXNUM_P((VALUE)*key)) {
-        iter_data->update_callback((VALUE *)key, iter_data->data);
-    }
-
-    return ST_CONTINUE;
-}
-
 static int
 vm_weak_table_gen_fields_foreach(st_data_t key, st_data_t value, st_data_t data)
 {
@@ -4356,17 +4075,6 @@ rb_gc_vm_weak_table_foreach(vm_table_foreach_callback_func callback,
         );
         break;
       }
-      case RB_GC_VM_ID2REF_TABLE: {
-        if (id2ref_tbl) {
-            st_foreach_with_replace(
-                id2ref_tbl,
-                vm_weak_table_id2ref_foreach,
-                vm_weak_table_id2ref_foreach_update,
-                (st_data_t)&foreach_data
-            );
-        }
-        break;
-      }
       case RB_GC_VM_GENERIC_FIELDS_TABLE: {
         st_table *generic_fields_tbl = rb_generic_fields_tbl_get();
         if (generic_fields_tbl) {
@@ -5870,8 +5578,6 @@ Init_GC(void)
 #endif
 
 #undef rb_intern
-    rb_gc_register_address(&id2ref_value);
-
     malloc_offset = gc_compute_malloc_offset();
 
     rb_mGC = rb_define_module("GC");
@@ -5883,8 +5589,6 @@ Init_GC(void)
     rb_define_module_function(rb_mObjSpace, "define_finalizer", define_final, -1);
     rb_define_module_function(rb_mObjSpace, "undefine_finalizer", undefine_final, 1);
 
-    rb_define_module_function(rb_mObjSpace, "_id2ref", os_id2ref, 1);
-
     rb_vm_register_special_exception(ruby_error_nomemory, rb_eNoMemError, "failed to allocate memory");
 
     rb_define_method(rb_cBasicObject, "__id__", rb_obj_id, 0);

gc/gc.h

@@ -48,7 +48,6 @@ enum rb_gc_vm_weak_tables {
     RB_GC_VM_CI_TABLE,
     RB_GC_VM_OVERLOADED_CME_TABLE,
     RB_GC_VM_GLOBAL_SYMBOLS_TABLE,
-    RB_GC_VM_ID2REF_TABLE,
     RB_GC_VM_GENERIC_FIELDS_TABLE,
     RB_GC_VM_FROZEN_STRINGS_TABLE,
     RB_GC_VM_WEAK_TABLE_COUNT

internal/gc.h

@@ -199,7 +199,6 @@ RUBY_ATTR_MALLOC void *rb_xcalloc_mul_add(size_t, size_t, size_t);
 void *rb_xrealloc_mul_add(const void *, size_t, size_t, size_t);
 RUBY_ATTR_MALLOC void *rb_xmalloc_mul_add_mul(size_t, size_t, size_t, size_t);
 RUBY_ATTR_MALLOC void *rb_xcalloc_mul_add_mul(size_t, size_t, size_t, size_t);
-void rb_gc_obj_id_moved(VALUE obj);
 void rb_gc_register_pinning_obj(VALUE obj);
 rb_execution_context_t *rb_gc_get_ec(void);
 

ractor.c

@@ -2059,8 +2059,6 @@ move_leave(VALUE obj, struct obj_traverse_replace_data *data)
         rb_replace_generic_ivar(data->replacement, obj);
     }
 
-    rb_gc_obj_id_moved(data->replacement);
-
     VALUE flags = T_OBJECT | FL_FREEZE | (RBASIC(obj)->flags & FL_PROMOTED);
 
     // Avoid mutations using bind_call, etc.

spec/ruby/core/objectspace/_id2ref_spec.rb

@@ -1,6 +1,6 @@
 require_relative '../../spec_helper'
 
-ruby_version_is "4.0" do
+ruby_version_is "4.0"..."4.1" do
   describe "ObjectSpace._id2ref" do
     it "is deprecated" do
       id = nil.object_id

test/ruby/test_objectspace.rb

@@ -2,77 +2,6 @@
 require 'test/unit'
 
 class TestObjectSpace < Test::Unit::TestCase
-  def self.deftest_id2ref(obj)
-    /:(\d+)/ =~ caller[0]
-    file = $`
-    line = $1.to_i
-    code = <<"End"
-    define_method("test_id2ref_#{line}") {\
-      o = EnvUtil.suppress_warning { ObjectSpace._id2ref(obj.object_id) }
-      assert_same(obj, o, "didn't round trip: \#{obj.inspect}");\
-    }
-End
-    eval code, binding, file, line
-  end
-
-  deftest_id2ref(-0x4000000000000001)
-  deftest_id2ref(-0x4000000000000000)
-  deftest_id2ref(-0x40000001)
-  deftest_id2ref(-0x40000000)
-  deftest_id2ref(-1)
-  deftest_id2ref(0)
-  deftest_id2ref(1)
-  deftest_id2ref(0x3fffffff)
-  deftest_id2ref(0x40000000)
-  deftest_id2ref(0x3fffffffffffffff)
-  deftest_id2ref(0x4000000000000000)
-  deftest_id2ref(:a)
-  deftest_id2ref(:abcdefghijilkjl)
-  deftest_id2ref(:==)
-  deftest_id2ref(Object.new)
-  deftest_id2ref(self)
-  deftest_id2ref(true)
-  deftest_id2ref(false)
-  deftest_id2ref(nil)
-
-  def test_id2ref_liveness
-    assert_normal_exit <<-EOS
-      ids = []
-      10.times{
-        1_000.times{
-          ids << 'hello'.object_id
-        }
-        objs = ids.map{|id|
-          begin
-            ObjectSpace._id2ref(id)
-          rescue RangeError
-            nil
-          end
-        }
-        GC.start
-        objs.each{|e| e.inspect}
-      }
-    EOS
-  end
-
-  def test_id2ref_invalid_argument
-    msg = /no implicit conversion/
-    assert_raise_with_message(TypeError, msg) { EnvUtil.suppress_warning { ObjectSpace._id2ref(nil) } }
-    assert_raise_with_message(TypeError, msg) { EnvUtil.suppress_warning { ObjectSpace._id2ref(false) } }
-    assert_raise_with_message(TypeError, msg) { EnvUtil.suppress_warning { ObjectSpace._id2ref(true) } }
-    assert_raise_with_message(TypeError, msg) { EnvUtil.suppress_warning { ObjectSpace._id2ref(:a) } }
-    assert_raise_with_message(TypeError, msg) { EnvUtil.suppress_warning { ObjectSpace._id2ref("0") } }
-    assert_raise_with_message(TypeError, msg) { EnvUtil.suppress_warning { ObjectSpace._id2ref(Object.new) } }
-  end
-
-  def test_id2ref_invalid_symbol_id
-    # RB_STATIC_SYM_P checks for static symbols by checking that the bottom
-    # 8 bits of the object is equal to RUBY_SYMBOL_FLAG, so we need to make
-    # sure that the bottom 8 bits remain unchanged.
-    msg = /is not a symbol id value/
-    assert_raise_with_message(RangeError, msg) { EnvUtil.suppress_warning { ObjectSpace._id2ref(:a.object_id + 256) } }
-  end
-
   def test_count_objects
     h = {}
     ObjectSpace.count_objects(h)
@@ -284,21 +213,6 @@ End
     end;
   end
 
-  def test_id2ref_table_build
-    assert_separately([], <<-End)
-      10.times do
-        Object.new.object_id
-      end
-
-      GC.start(immediate_mark: false)
-
-      obj = Object.new
-      EnvUtil.suppress_warning do
-        assert_equal obj, ObjectSpace._id2ref(obj.object_id)
-      end
-    End
-  end
-
   def test_each_object_singleton_class
     assert_separately([], <<-End)
       class C