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

A simple benchmark-ips script starts the finaliser thread and object sharing #2264

Open
chrisseaton opened this issue Feb 20, 2021 · 6 comments
Assignees

Comments

@chrisseaton
Copy link
Collaborator

<internal:core> core/truffle/ffi/pointer.rb:255:in `autorelease='
<internal:core> core/truffle/ffi/pointer.rb:255:in `new'
<internal:core> core/truffle/ffi/pointer.rb:262:in `from_string'
<internal:core> core/posix.rb:330:in `block (2 levels) in with_array_of_strings_pointer'
<internal:core> core/posix.rb:329:in `map'
<internal:core> core/posix.rb:329:in `block in with_array_of_strings_pointer'
<internal:core> core/truffle/ffi/pointer.rb:250:in `new'
<internal:core> core/posix.rb:328:in `with_array_of_strings_pointer'
<internal:core> core/truffle/process_operations.rb:488:in `block in posix_spawnp'
<internal:core> core/posix.rb:322:in `block in with_array_of_ints'
<internal:core> core/truffle/ffi/pointer.rb:250:in `new'
<internal:core> core/posix.rb:320:in `with_array_of_ints'
<internal:core> core/truffle/process_operations.rb:487:in `posix_spawnp'
<internal:core> core/truffle/process_operations.rb:418:in `spawn'
<internal:core> core/truffle/process_operations.rb:84:in `spawn'
<internal:core> core/io.rb:862:in `popen'
<internal:core> core/kernel.rb:178:in ``'
/Users/chrisseaton/src/github.com/Shopify/truffleruby/mxbuild/truffleruby-jvm/languages/ruby/lib/truffle/truffle/openssl-prefix.rb:14:in `<top (required)>'
<internal:core> core/kernel.rb:260:in `require'
/Users/chrisseaton/src/github.com/Shopify/truffleruby/mxbuild/truffleruby-jvm/languages/ruby/lib/truffle/rbconfig.rb:99:in `<module:RbConfig>'
/Users/chrisseaton/src/github.com/Shopify/truffleruby/mxbuild/truffleruby-jvm/languages/ruby/lib/truffle/rbconfig.rb:37:in `<top (required)>'
<internal:core> core/kernel.rb:260:in `require'
/Users/chrisseaton/src/github.com/Shopify/truffleruby/mxbuild/truffleruby-jvm/languages/ruby/lib/mri/rubygems.rb:9:in `<top (required)>'
<internal:core> core/kernel.rb:260:in `require'
/Users/chrisseaton/src/github.com/Shopify/truffleruby/mxbuild/truffleruby-jvm/languages/ruby/lib/patches/rubygems.rb:6:in `<top (required)>'
<internal:core> core/kernel.rb:234:in `gem_original_require'
<internal:core> core/kernel.rb:264:in `require'
/Users/chrisseaton/xxxx/class-variables/benchmarks.rb:1:in `<main>'

We should possibly:

  • check if this makes sense in the first place - why is RubyGems doing this?
  • not use the Ruby finaliser thread for something that can be handled outside of a context
@eregon eregon self-assigned this Feb 21, 2021
@eregon
Copy link
Member

eregon commented Feb 21, 2021

Right, that explains why it only happens on macOS then, it's due to the subprocesses of https://github.com/oracle/truffleruby/blob/634c0ae2517d04261b05e357b79da009eeac62ff/lib/truffle/truffle/openssl-prefix.rb.
I'll look into it (running an extra subcommand when loading RubyGems to find libssl also seems suboptimal).

graalvmbot pushed a commit that referenced this issue Feb 22, 2021
@eregon
Copy link
Member

eregon commented Feb 23, 2021

8ca9852 avoids creating autorelease pointers for spawning subprocesses.

It would be interesting to know how long brew --prefix takes to see if it's worth optimizing for the common case of Homebrew being in /usr/local.
@chrisseaton Could you measure that?

Regarding having Pointer finalizers without an extra thread, that's something that probably makes more sense in Truffle, and NFI has a NativeAllocation that does something very similar.

With the new Truffle safepoints it might also be possible to simply run all finalizers on the main Ruby thread, but I'm unsure if it's a good idea or not.

@chrisseaton
Copy link
Collaborator Author

Almost a second!

chrisseaton@Chris-Seatons-MacBook-Pro graal % disable -r time      
chrisseaton@Chris-Seatons-MacBook-Pro graal % time -p brew --prefix openssl
/usr/local/opt/openssl@1.1
real         0.89
user         0.48
sys          0.40
chrisseaton@Chris-Seatons-MacBook-Pro graal % time -p brew --prefix openssl
/usr/local/opt/openssl@1.1
real         0.86
user         0.47
sys          0.38
chrisseaton@Chris-Seatons-MacBook-Pro graal % time -p brew --prefix openssl
/usr/local/opt/openssl@1.1
real         0.85
user         0.47
sys          0.37

@chrisseaton
Copy link
Collaborator Author

chrisseaton@Chris-Seatons-MacBook-Pro ~ % time -p brew --prefix
/usr/local
real         0.02
user         0.01
sys          0.01
chrisseaton@Chris-Seatons-MacBook-Pro ~ % time -p brew --prefix
/usr/local
real         0.02
user         0.01
sys          0.01
chrisseaton@Chris-Seatons-MacBook-Pro ~ % time -p brew --prefix
/usr/local
real         0.03
user         0.01
sys          0.01

@eregon
Copy link
Member

eregon commented Feb 23, 2021

Right, so that's more like 20-30ms, seems worth optimizing the common case.

@eregon
Copy link
Member

eregon commented Feb 24, 2021

3b05154 avoids that extra subprocess if Homebrew is in /usr/local.

wildmaples pushed a commit to Shopify/truffleruby that referenced this issue Mar 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants