ruby-changes:64201
From: Kenta <ko1@a...>
Date: Wed, 16 Dec 2020 13:44:18 +0900 (JST)
Subject: [ruby-changes:64201] 1bafb3cb47 (master): [memory_view] Make MemoryView API Ractor-safe (#3911)
https://git.ruby-lang.org/ruby.git/commit/?id=1bafb3cb47 From 1bafb3cb47649037cc93fec97503c18a864e3983 Mon Sep 17 00:00:00 2001 From: Kenta Murata <mrkn@u...> Date: Wed, 16 Dec 2020 13:43:56 +0900 Subject: [memory_view] Make MemoryView API Ractor-safe (#3911) * memory_view.c: make Ractor-safe * test/ruby/test_memory_view.rb: Add test_ractor * memory_view: fix typo * memory_view.c: Use st_update in unregster_exported_object * memory_view: update dependency diff --git a/common.mk b/common.mk index 1dd2667..e5e0683 100644 --- a/common.mk +++ b/common.mk @@ -8018,6 +8018,7 @@ memory_view.$(OBJEXT): {$(VPATH)}backward/2/stdalign.h https://github.com/ruby/ruby/blob/trunk/common.mk#L8018 memory_view.$(OBJEXT): {$(VPATH)}backward/2/stdarg.h memory_view.$(OBJEXT): {$(VPATH)}config.h memory_view.$(OBJEXT): {$(VPATH)}constant.h +memory_view.$(OBJEXT): {$(VPATH)}debug_counter.h memory_view.$(OBJEXT): {$(VPATH)}defines.h memory_view.$(OBJEXT): {$(VPATH)}id_table.h memory_view.$(OBJEXT): {$(VPATH)}intern.h @@ -8165,8 +8166,11 @@ memory_view.$(OBJEXT): {$(VPATH)}internal/xmalloc.h https://github.com/ruby/ruby/blob/trunk/common.mk#L8166 memory_view.$(OBJEXT): {$(VPATH)}memory_view.c memory_view.$(OBJEXT): {$(VPATH)}memory_view.h memory_view.$(OBJEXT): {$(VPATH)}missing.h +memory_view.$(OBJEXT): {$(VPATH)}node.h memory_view.$(OBJEXT): {$(VPATH)}st.h memory_view.$(OBJEXT): {$(VPATH)}subst.h +memory_view.$(OBJEXT): {$(VPATH)}vm_debug.h +memory_view.$(OBJEXT): {$(VPATH)}vm_sync.h miniinit.$(OBJEXT): $(CCAN_DIR)/check_type/check_type.h miniinit.$(OBJEXT): $(CCAN_DIR)/container_of/container_of.h miniinit.$(OBJEXT): $(CCAN_DIR)/list/list.h diff --git a/ext/-test-/memory_view/memory_view.c b/ext/-test-/memory_view/memory_view.c index 61d0246..c85ba6f 100644 --- a/ext/-test-/memory_view/memory_view.c +++ b/ext/-test-/memory_view/memory_view.c @@ -370,6 +370,7 @@ mdview_aref(VALUE obj, VALUE indices_v) https://github.com/ruby/ruby/blob/trunk/ext/-test-/memory_view/memory_view.c#L370 void Init_memory_view(void) { + rb_ext_ractor_safe(true); VALUE mMemoryViewTestUtils = rb_define_module("MemoryViewTestUtils"); rb_define_module_function(mMemoryViewTestUtils, "available?", memory_view_available_p, 1); diff --git a/memory_view.c b/memory_view.c index 5d6ef14..7a2a518 100644 --- a/memory_view.c +++ b/memory_view.c @@ -11,6 +11,7 @@ https://github.com/ruby/ruby/blob/trunk/memory_view.c#L11 #include "internal/variable.h" #include "internal/util.h" #include "ruby/memory_view.h" +#include "vm_sync.h" #if SIZEOF_INTPTR_T == SIZEOF_LONG_LONG # define INTPTR2NUM LL2NUM @@ -30,6 +31,7 @@ https://github.com/ruby/ruby/blob/trunk/memory_view.c#L31 // Exported Object Registry +static st_table *exported_object_table = NULL; VALUE rb_memory_view_exported_object_registry = Qundef; static int @@ -42,16 +44,18 @@ exported_object_registry_mark_key_i(st_data_t key, st_data_t value, st_data_t da https://github.com/ruby/ruby/blob/trunk/memory_view.c#L44 static void exported_object_registry_mark(void *ptr) { - st_table *table = ptr; - st_foreach(table, exported_object_registry_mark_key_i, 0); + // Don't use RB_VM_LOCK_ENTER here. It is unnecessary during GC. + st_foreach(exported_object_table, exported_object_registry_mark_key_i, 0); } static void exported_object_registry_free(void *ptr) { - st_table *table = ptr; - st_clear(table); - st_free_table(table); + // Note that calling RB_VM_LOCK_ENTER here is unnecessary now. + // But it may be changed in the future. + st_clear(exported_object_table); + st_free_table(exported_object_table); + exported_object_table = NULL; } const rb_data_type_t rb_memory_view_exported_object_registry_data_type = { @@ -64,33 +68,11 @@ const rb_data_type_t rb_memory_view_exported_object_registry_data_type = { https://github.com/ruby/ruby/blob/trunk/memory_view.c#L68 0, 0, RUBY_TYPED_FREE_IMMEDIATELY }; -static void -init_exported_object_registry(void) -{ - if (rb_memory_view_exported_object_registry != Qundef) { - return; - } - - st_table *table = rb_init_identtable(); - VALUE obj = TypedData_Wrap_Struct( - 0, &rb_memory_view_exported_object_registry_data_type, table); - rb_gc_register_mark_object(obj); - rb_memory_view_exported_object_registry = obj; -} - -static inline st_table * -get_exported_object_table(void) -{ - st_table *table; - TypedData_Get_Struct(rb_memory_view_exported_object_registry, st_table, - &rb_memory_view_exported_object_registry_data_type, - table); - return table; -} - static int -update_exported_object_ref_count(st_data_t *key, st_data_t *val, st_data_t arg, int existing) +exported_object_add_ref(st_data_t *key, st_data_t *val, st_data_t arg, int existing) { + ASSERT_vm_locking(); + if (existing) { *val += 1; } @@ -100,39 +82,34 @@ update_exported_object_ref_count(st_data_t *key, st_data_t *val, st_data_t arg, https://github.com/ruby/ruby/blob/trunk/memory_view.c#L82 return ST_CONTINUE; } -static void -register_exported_object(VALUE obj) +static int +exported_object_dec_ref(st_data_t *key, st_data_t *val, st_data_t arg, int existing) { - if (rb_memory_view_exported_object_registry == Qundef) { - init_exported_object_registry(); - } + ASSERT_vm_locking(); - st_table *table = get_exported_object_table(); + if (existing) { + *val -= 1; + if (*val == 0) { + return ST_DELETE; + } + } + return ST_CONTINUE; +} - st_update(table, (st_data_t)obj, update_exported_object_ref_count, 0); +static void +register_exported_object(VALUE obj) +{ + RB_VM_LOCK_ENTER(); + st_update(exported_object_table, (st_data_t)obj, exported_object_add_ref, 0); + RB_VM_LOCK_LEAVE(); } static void unregister_exported_object(VALUE obj) { - if (rb_memory_view_exported_object_registry == Qundef) { - return; - } - - st_table *table = get_exported_object_table(); - - st_data_t count; - if (!st_lookup(table, (st_data_t)obj, &count)) { - return; - } - - if (--count == 0) { - st_data_t key = (st_data_t)obj; - st_delete(table, &key, &count); - } - else { - st_insert(table, (st_data_t)obj, count); - } + RB_VM_LOCK_ENTER(); + st_update(exported_object_table, (st_data_t)obj, exported_object_dec_ref, 0); + RB_VM_LOCK_LEAVE(); } // MemoryView @@ -876,5 +853,15 @@ rb_memory_view_release(rb_memory_view_t* view) https://github.com/ruby/ruby/blob/trunk/memory_view.c#L853 void Init_MemoryView(void) { + exported_object_table = rb_init_identtable(); + + // exported_object_table is referred through rb_memory_view_exported_object_registry + // in -test-/memory_view extension. + VALUE obj = TypedData_Wrap_Struct( + 0, &rb_memory_view_exported_object_registry_data_type, + exported_object_table); + rb_gc_register_mark_object(obj); + rb_memory_view_exported_object_registry = obj; + id_memory_view = rb_intern_const("__memory_view__"); } diff --git a/test/ruby/test_memory_view.rb b/test/ruby/test_memory_view.rb index 9a6c834..ed2f53c 100644 --- a/test/ruby/test_memory_view.rb +++ b/test/ruby/test_memory_view.rb @@ -320,4 +320,22 @@ class TestMemoryView < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_memory_view.rb#L320 assert_equal([-1, -2], mv[[1, 0]]) assert_equal([-7, -8], mv[[1, 3]]) end + + def test_ractor + assert_in_out_err([], <<-"end;", ["[5, 6]", "[-7, -8]"], []) + require "-test-/memory_view" + require "rbconfig/sizeof" + $VERBOSE = nil + r = Ractor.new RbConfig::SIZEOF["short"] do |sizeof_short| + buf = [ 1, 2, 3, 4, 5, 6, 7, 8, + -1, -2, -3, -4, -5, -6, -7, -8].pack("s*") + shape = [2, 4] + strides = [4*sizeof_short*2, sizeof_short*2] + mv = MemoryViewTestUtils::MultiDimensionalView.new(buf, "ss", shape, strides) + p mv[[0, 2]] + mv[[1, 3]] + end + p r.take + end; + end end -- cgit v0.10.2 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/