From 0c36ba53192c5a0d245c9b626e4346a32d7d144e Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Mon, 23 May 2022 17:31:14 -0400 Subject: [PATCH] Define unsupported GC compaction methods as rb_f_notimplement Fixes [Bug #18779] Define the following methods as `rb_f_notimplement` on unsupported platforms: - GC.compact - GC.auto_compact - GC.auto_compact= - GC.latest_compact_info - GC.verify_compaction_references This change allows users to call `GC.respond_to?(:compact)` to properly test for compaction support. Previously, it was necessary to invoke `GC.compact` or `GC.verify_compaction_references` and check if those methods raised `NotImplementedError` to determine if compaction was supported. This follows the precedent set for other platform-specific methods. For example, in `process.c` for methods such as `Process.fork`, `Process.setpgid`, and `Process.getpriority`. --- gc.c | 30 ++++++++++++++++--- test/ruby/test_gc_compact.rb | 58 ++++++++++++++++++++++++++---------- 2 files changed, 69 insertions(+), 19 deletions(-) diff --git a/gc.c b/gc.c index 3f1a94ecb0..c9c2bd8ad4 100644 --- a/gc.c +++ b/gc.c @@ -9671,6 +9671,7 @@ gc_move(rb_objspace_t *objspace, VALUE scan, VALUE free, size_t slot_size) return (VALUE)src; } +#if GC_COMPACTION_SUPPORTED static int compare_free_slots(const void *left, const void *right, void *dummy) { @@ -9719,6 +9720,7 @@ gc_sort_heap_by_empty_slots(rb_objspace_t *objspace) free(page_list); } } +#endif static void gc_ref_update_array(rb_objspace_t * objspace, VALUE v) @@ -10410,6 +10412,7 @@ gc_update_references(rb_objspace_t *objspace) gc_update_table_refs(objspace, finalizer_table); } +#if GC_COMPACTION_SUPPORTED /* * call-seq: * GC.latest_compact_info -> {:considered=>{:T_CLASS=>11}, :moved=>{:T_CLASS=>11}} @@ -10446,7 +10449,11 @@ gc_compact_stats(VALUE self) return h; } +#else +# define gc_compact_stats rb_f_notimplement +#endif +#if GC_COMPACTION_SUPPORTED static void root_obj_check_moved_i(const char *category, VALUE obj, void *data) { @@ -10508,6 +10515,10 @@ heap_check_moved_i(void *vstart, void *vend, size_t stride, void *data) * * This method is implementation specific and not expected to be implemented * in any implementation besides MRI. + * + * To test whether GC compaction is supported, use the idiom: + * + * GC.respond_to?(:compact) */ static VALUE gc_compact(VALUE self) @@ -10517,7 +10528,11 @@ gc_compact(VALUE self) return gc_compact_stats(self); } +#else +# define gc_compact rb_f_notimplement +#endif +#if GC_COMPACTION_SUPPORTED /* * call-seq: * GC.verify_compaction_references(toward: nil, double_heap: false) -> hash @@ -10586,6 +10601,9 @@ gc_verify_compaction_references(int argc, VALUE *argv, VALUE self) return gc_compact_stats(self); } +#else +# define gc_verify_compaction_references rb_f_notimplement +#endif VALUE rb_gc_start(void) @@ -11173,6 +11191,7 @@ gc_disable(rb_execution_context_t *ec, VALUE _) return rb_gc_disable(); } +#if GC_COMPACTION_SUPPORTED /* * call-seq: * GC.auto_compact = flag @@ -11194,14 +11213,14 @@ gc_set_auto_compact(VALUE _, VALUE v) } #endif -#if !GC_COMPACTION_SUPPORTED - rb_raise(rb_eNotImpError, "Automatic compaction isn't available on this platform"); -#endif - ruby_enable_autocompact = RTEST(v); return v; } +#else +# define gc_set_auto_compact rb_f_notimplement +#endif +#if GC_COMPACTION_SUPPORTED /* * call-seq: * GC.auto_compact -> true or false @@ -11213,6 +11232,9 @@ gc_get_auto_compact(VALUE _) { return RBOOL(ruby_enable_autocompact); } +#else +# define gc_get_auto_compact rb_f_notimplement +#endif static int get_envparam_size(const char *name, size_t *default_value, size_t lower_bound) diff --git a/test/ruby/test_gc_compact.rb b/test/ruby/test_gc_compact.rb index 16b15b39ac..da0023e6f3 100644 --- a/test/ruby/test_gc_compact.rb +++ b/test/ruby/test_gc_compact.rb @@ -9,14 +9,7 @@ if RUBY_PLATFORM =~ /s390x/ end class TestGCCompact < Test::Unit::TestCase - module SupportsCompact - def setup - omit "autocompact not supported on this platform" unless supports_auto_compact? - super - end - - private - + module CompactionSupportInspector def supports_auto_compact? return false if /wasm/ =~ RUBY_PLATFORM return true unless defined?(Etc::SC_PAGE_SIZE) @@ -31,10 +24,19 @@ class TestGCCompact < Test::Unit::TestCase end end - include SupportsCompact + module OmitUnlessCompactSupported + include CompactionSupportInspector + + def setup + omit "autocompact not supported on this platform" unless supports_auto_compact? + super + end + end + + include OmitUnlessCompactSupported class AutoCompact < Test::Unit::TestCase - include SupportsCompact + include OmitUnlessCompactSupported def test_enable_autocompact before = GC.auto_compact @@ -88,13 +90,39 @@ class TestGCCompact < Test::Unit::TestCase end end - def os_page_size - return true unless defined?(Etc::SC_PAGE_SIZE) + class CompactMethodsNotImplemented < Test::Unit::TestCase + include CompactionSupportInspector + + def assert_not_implemented(method, *args) + omit "autocompact is supported on this platform" if supports_auto_compact? + + assert_raise(NotImplementedError) { GC.send(method, *args) } + refute(GC.respond_to?(method), "GC.#{method} should be defined as rb_f_notimplement") + end + + def test_gc_compact_not_implemented + assert_not_implemented(:compact) + end + + def test_gc_auto_compact_get_not_implemented + assert_not_implemented(:auto_compact) + end + + def test_gc_auto_compact_set_not_implemented + assert_not_implemented(:auto_compact=, true) + end + + def test_gc_latest_compact_info_not_implemented + assert_not_implemented(:latest_compact_info) + end + + def test_gc_verify_compaction_references_not_implemented + assert_not_implemented(:verify_compaction_references) + end end - def setup - omit "autocompact not supported on this platform" unless supports_auto_compact? - super + def os_page_size + return true unless defined?(Etc::SC_PAGE_SIZE) end def test_gc_compact_stats