Bug #18829: GC_COMPACTION_SUPPORTED macro should be set and detected automatically. - Ruby master - Ruby Issue Tracking System

Actions

Copy link

Bug #18829

closed

GC_COMPACTION_SUPPORTED macro should be set and detected automatically.

Added by jprokop (Jarek Prokop) almost 2 years ago. Updated almost 2 years ago.

Status:

Closed

Assignee:

Target version:

ruby -v:

ruby 3.2.0dev (2022-06-14T01:21:55Z master 048f14221c) [powerpc64le-linux]

Backport:

2.7: DONTNEED, 3.0: REQUIRED, 3.1: REQUIRED

[ruby-core:108908]

Description

After backporting the patch [0] for GC compaction support from https://github.com/ruby/ruby/pull/5934 (for bug https://bugs.ruby-lang.org/issues/18779), I found that there is no mechanism to automatically define the GC_COMPACTION_SUPPORTED based on the platform.

This does not seem to be a major issue with the Ruby master branch as the Ruby GC page size is bumped up to 64K so platforms like ppc64le do not struggle anymore, however, if the Ruby GC page size is set to 16K as is the case with Ruby 3.1 for example, then the compaction is enabled even in cases when it should not be.

For master I can reach a failure if I set the Ruby GC page size to 16K by editing macro in gc.c (https://github.com/ruby/ruby/blob/master/gc.c#L863) to:

#define HEAP_PAGE_ALIGN_LOG 14

As there are no compile-time checks around this, after compiling ruby and using the following script I get an exception.
script:

if GC.respond_to?(:compact)
  GC.verify_compaction_references(double_heap: true, toward: :empty)
end

100_000.times do |i|
  puts "i: #{i}, count: #{count}"
end

Running it raises an exception even though the code is in a guard clause that should prevent this:

~/.rubies/ruby-master/bin/ruby ./reproducer.rb 
<internal:gc>:251:in `verify_compaction_references': Compaction isn't available on this platform (NotImplementedError)
	from ./reproducer.rb:4:in `<main>'

Right now, our workaround is to define the macro GC_COMPACTION_SUPPORTED only on platforms that we know do support compaction. Ideally, this should not be required and the configuration script or the gc.c file should take care of defining the macro correctly.

[0] https://src.fedoraproject.org/fork/jackorp/rpms/ruby/blob/1e34def591d4615bde83a28ff6cd336aa5879a80/f/ruby-3.2.0-define-unsupported-gc-compaction-methods-as-rb_f_notimplement.patch

Actions

Copy link

#1 [ruby-core:108916]

Updated by jeremyevans0 (Jeremy Evans) almost 2 years ago

Status changed from Open to Closed
Backport changed from 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN to 2.7: DONTNEED, 3.0: REQUIRED, 3.1: REQUIRED

Since this problem does not occur in the master branch (due to the increased page size), it seems like the bug has already been fixed/worked around. It seems unlikely we would backport the increased page size to older Ruby versions, and we don't generally apply patches to release branches that are not backported from master (it only happens in exceptional cases, and this case doesn't appear to warrant it).

I'm marking this for backport to supported versions, but the odds of a fix being backported without you providing a patch seem low. Even with a patch, it would be up to the branch maintainer whether to apply it.

Another possibility would be adding a patch to the master branch for this, and then asking for that patch to be backported, but if this is no longer an issue in the master branch due to the increased page size, I would be against applying such a patch.

Actions

Copy link

#2 [ruby-core:108918]