Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test/helper.rb: ignore GC compaction on unsupported platforms #2532

Merged
merged 1 commit into from
May 9, 2022

Conversation

terceiro
Copy link
Contributor

@terceiro terceiro commented May 7, 2022

For example, ruby 3.0 on Debian ppc64el architecture does not support GC
compaction. When running the tests, every 20th test crashes like this:

Error:
Minitest::Result#test_parsing_attribute_namespace:
NotImplementedError: Compaction isn't available on this platform
    <internal:gc>:213:in `compact'
    /home/terceiro/ruby-nokogiri-1.13.5+dfsg/test/helper.rb:123:in `teardown'

@flavorjones
Copy link
Member

flavorjones commented May 7, 2022

If you look at the method initialize_nokogiri_test_gc_level in test/helper.rb you can see that it checks if GC.compact and GC.verify_compaction_references are defined? before deciding to set the compaction verification. Can you help me understand why that's not working?

@flavorjones
Copy link
Member

I'd prefer a PR that did the appropriate checks in initialize_nokogiri_test_gc_level than wrapped these calls in rescue blocks, if that's possible.

For example, ruby 3.0 on Debian ppc64el architecture does not support GC
compaction. When running the tests, every 20th test crashes like this:

> Error:
> Minitest::Result#test_parsing_attribute_namespace:
> NotImplementedError: Compaction isn't available on this platform
>     <internal:gc>:213:in `compact'
>     /home/terceiro/ruby-nokogiri-1.13.5+dfsg/test/helper.rb:123:in `teardown'
@terceiro
Copy link
Contributor Author

terceiro commented May 9, 2022

If you look at the method initialize_nokogiri_test_gc_level in test/helper.rb you can see that it checks if GC.compact and GC.verify_compaction_references are defined? before deciding to set the compaction verification. Can you help me understand why that's not working?

When compaction is not supported, those methods are defined, but raise NotImplementedError when called:

irb(main):001:0> defined?(GC.compact)
=> "method"
irb(main):002:0> GC.compact
<internal:gc>:213:in `compact': Compaction isn't available on this platform (NotImplementedError)
	from (irb):2:in `<main>'
	from /usr/lib/ruby/gems/3.0.0/gems/irb-1.3.5/exe/irb:11:in `<top (required)>'
	from /usr/bin/irb:23:in `load'
	from /usr/bin/irb:23:in `<main>'

I'd prefer a PR that did the appropriate checks in initialize_nokogiri_test_gc_level than wrapped these calls in rescue blocks, if that's possible.

For that, you would need to actuall call GC.compact at the beginning. I just sent an updated patch, let me know what you think.

@stevecheckoway
Copy link
Contributor

For that, you would need to actuall call GC.compact at the beginning. I just sent an updated patch, let me know what you think.

This seems like a reasonable approach.

@flavorjones
Copy link
Member

I agree, this looks good! I've just kicked off CI.

I'm wondering if it would be worth running CI on ppc64le (using qemu like we do for native gem installations) ...

@stevecheckoway
Copy link
Contributor

Little endian PowerPC seems like a pretty esoteric platform. Maybe something that runs periodically but not on every commit?

@flavorjones
Copy link
Member

I'm curious if there's a way I can easily reproduce what you're seeing? I'm trying the platform=linux/ppc64le docker container for ruby:3.0 (and multiarch/qemu-user-static to emulate that hardware), but it seems to be working OK (using current main at 199434c):

warnings: []
nokogiri:
  version: 1.14.0.dev
  cppflags:
  - "-I/root/nokogiri/ext/nokogiri"
  ldflags: []
ruby:
  version: 3.0.4
  platform: powerpc64le-linux
  gem_platform: powerpc64le-linux
  description: ruby 3.0.4p208 (2022-04-12 revision 3fa771dded) [powerpc64le-linux]
  engine: ruby
libxml:
  source: system
  memory_management: ruby
  iconv_enabled: true
  compiled: 2.9.10
  loaded: 2.9.10
libxslt:
  source: system
  datetime_enabled: true
  compiled: 1.1.34
  loaded: 1.1.34
other_libraries:
  libgumbo: 1.0.0-nokogiri

# Running tests with run options --seed 63270:

/root/nokogiri/test/helper.rb:157: NOKOGIRI_TEST_GC_LEVEL: compact
........................................................................................................................................................................................................................................S....................................S........................................S....................................................................................................................................................................................S.......................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................S.................................................SSS.................................................................................................................................S..........................................................S.......S.....S....S................................S.....................S......S.........S........S.......S.............................................................................................................................................................................................................................................................................................................^Z

@flavorjones
Copy link
Member

Ah, never mind, I compiled a custom version of ruby with GC_COMPACTION_SUPPORTED set to 0 and I'm testing it that way.

@flavorjones
Copy link
Member

@terceiro Thanks for this! Merging.

@flavorjones flavorjones merged commit a00e0d4 into sparklemotion:main May 9, 2022
@flavorjones
Copy link
Member

Note for posterity, this was reported upstream in https://bugs.ruby-lang.org/issues/18779

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants