Skip to content

Commit

Permalink
Define unsupported GC compaction methods as rb_f_notimplement
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
flavorjones authored and tenderlove committed May 24, 2022
1 parent 0de1495 commit 0c36ba5
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 19 deletions.
30 changes: 26 additions & 4 deletions gc.c
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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}}
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down
58 changes: 43 additions & 15 deletions test/ruby/test_gc_compact.rb
Expand Up @@ -9,14 +9,7 @@
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)
Expand All @@ -31,10 +24,19 @@ def supports_auto_compact?
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
Expand Down Expand Up @@ -88,13 +90,39 @@ def test_implicit_compaction_does_something
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
Expand Down

0 comments on commit 0c36ba5

Please sign in to comment.