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

Turn on --enable-shared by default for all supported MRI Rubies #1368

Merged
merged 1 commit into from Oct 30, 2019

Conversation

mislav
Copy link
Member

@mislav mislav commented Oct 25, 2019

Some gems need the host ruby to have shared library enabled. Currently, we're not aware of any potential downsides to having this enabled by default.

Fixes #35, ref. #921

Should we support a flag to disable shared, i.e. to override the default?

Some gems need the host ruby to have shared library enabled. Currently,
we're not aware of any potential downsides to having this enabled by
default.

> Shared libraries are libraries that are loaded by programs when they
> start. When a shared library is installed properly, all programs that
> start afterwards automatically use the new shared library.
> [...] if you have a program that needs a shared library (in this case ruby
> libruby.so.2.2.0 or similar) the program will fail if it doesn't exist.
Copy link
Member

@hsbt hsbt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no objection to this change. If we have some issues, we can revert this.

@eregon
Copy link
Member

eregon commented Oct 26, 2019

As mentioned in #35 (comment), one potential concern is that shared library might be a bit slower. I do remember observing that a long time ago on macOS.

I'm measuring on Linux with OptCarrot, ran from the TruffleRuby repository to use a simple harness.
I added

puts RUBY_DESCRIPTION
p RbConfig::CONFIG["ENABLE_SHARED"]

on top of bench/optcarrot/optcarrot.rb to see extra info.

$ for i in 1 2 3; do chruby shared; ruby bench/benchmark --simple bench/optcarrot/optcarrot.rb; chruby ruby-2.6.5; ruby bench/benchmark --simple bench/optcarrot/optcarrot.rb; done
ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-linux]
"yes"
optcarrot
50.317849048549355
46.39506283639085
45.3602131093604
45.422056204727284
45.490662973278
45.576647549137874
45.4174647094598
45.451302205638655
45.44400373090564
45.447830055702674
45.58365060075772
ruby bench/benchmark --simple bench/optcarrot/optcarrot.rb  10.55s user 0.02s system 99% cpu 10.589 total
ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-linux]
"no"
optcarrot
53.59360531494514
49.501233047689716
48.54883013844154
48.43964284875337
48.663360980187946
48.67714745765809
48.60241192643102
48.63888912422494
46.314266437076455
47.180858327799704
48.3996763881562
ruby bench/benchmark --simple bench/optcarrot/optcarrot.rb  10.51s user 0.02s system 99% cpu 10.546 total
ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-linux]
"yes"
optcarrot
53.09301594053763
46.633920380454136
45.51933449076868
45.41185702462387
45.68761125054893
46.16561365049164
46.20199837108337
46.04633758054757
46.15292642942895
45.924060249625036
46.169834175617105
ruby bench/benchmark --simple bench/optcarrot/optcarrot.rb  10.60s user 0.03s system 99% cpu 10.640 total
ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-linux]
"no"
optcarrot
55.60010359206679
48.80027288557057
47.825768477669726
47.9078629130865
47.88064158827453
48.121530347440604
47.981559343299104
47.975414735286265
47.973457049800366
47.96195988788927
48.06766120171843
ruby bench/benchmark --simple bench/optcarrot/optcarrot.rb  10.50s user 0.02s system 99% cpu 10.527 total
ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-linux]
"yes"
optcarrot
54.04681616263355
46.48391264477206
45.22067693778289
45.26908783297774
45.95690699674258
46.27918418699485
46.22674430726679
46.10003966781029
45.94780448882098
46.0890340524008
46.14758829385743
ruby bench/benchmark --simple bench/optcarrot/optcarrot.rb  10.63s user 0.02s system 99% cpu 10.664 total
ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-linux]
"no"
optcarrot
48.71543132001456
48.22709046980649
47.19250407016516
47.21186062789364
47.17361934542194
47.353863250130466
47.21085026692812
47.14130223501183
47.266345763868436
47.21551559880039
47.22466743560537
ruby bench/benchmark --simple bench/optcarrot/optcarrot.rb  10.49s user 0.02s system 99% cpu 10.524 total

There is some noise, notably because I'm running this on my laptop, but it seems to indicate the difference, although it's not enough data to "prove" it.
It's not a very large difference (2-6% in those measurements), but it seems to not be in favor of --enable-shared consistently.
For the record, this is with two builds of MRI 2.6.5 installed with ruby-install.

@eregon
Copy link
Member

eregon commented Oct 26, 2019

Here is a run with a bit less noise:
https://gist.github.com/eregon/2e41061391979f8ab19010179d7dc173
All 3 runs are 46 FPS on --enable-shared and all 3 runs are 47 FPS without.

@mislav
Copy link
Member Author

mislav commented Oct 28, 2019

@eregon Thank you for checking!

Unless performance with --enable-shared is abysmal and would noticeably affect people's experience when developing with Ruby, I vote that we enable this. If someone comes to us and says that this change to ruby-build led to their production environment reporting a % of slowdown in their Ruby processes, we can always make a case that ruby-build is not responsible for performance-tuning your production Rubies— one should maintain custom definitions for their production environments to enable fine-tuning.

@mislav
Copy link
Member Author

mislav commented Oct 28, 2019

@hsbt Let's leave this open for a day or two and merge if there is no evidence of significant regression. I thought that we might want to introduce a flag to disable shared, but we can also instruct people to copy the build definition to a separate file and remove enable_shared from that definition. It's a bit of a hassle, but hopefully people won't have to do it too often

@eregon
Copy link
Member

eregon commented Oct 28, 2019

@mislav I think it would be safer to introduce a flag or document a way to build without the shared library (maybe -- --disable-shared works?) as an easy opt-out, should any problem arise.

One more thing on this subject is RubyGems puts C extensions files for enable-shared Rubies in a different directory (which is good), see postmodern/chruby#410 (comment)
That might lead to warnings like Ignoring msgpack-1.3.0 because its extensions are not built. Try: gem pristine msgpack --version 1.3.0 if somehow mixing already-installed non-shared-library gems and a --enable-shared Ruby.
Given that rbenv doesn't set GEM_HOME, this should only happen for gems installed with gem install --user or to a specific path (e.g., bundler --install path) but not for gems installed in the default gem directory.

@mislav
Copy link
Member Author

mislav commented Oct 30, 2019

@eregon Thank you for that bit of info!

Merging this as-is. Let's see if it causes trouble. 🔥🔥 #famouslastwords

@mislav mislav merged commit ef93e25 into master Oct 30, 2019
@mislav mislav deleted the enable-shared branch October 30, 2019 17:10
@ankane ankane mentioned this pull request Oct 31, 2019
@ankane
Copy link

ankane commented Oct 31, 2019

Thanks @mislav, @hsbt, and @eregon!

jasonkarns added a commit to nodenv/node-build that referenced this pull request Nov 7, 2019
ruby-build 20191030

* Turn on `--enable-shared` by default for all supported MRI Rubies. rbenv/ruby-build#1368

  Some gems need the host ruby to have shared library enabled. Currently, we're not aware of any potential downsides to having this enabled by default.

* tag 'ruby-build/v20191030':
  ruby-build 20191030
  Turn on `--enable-shared` by default for all supported MRI Rubies
  Correct a typo in ruby-build
metalefty added a commit to metalefty/ruby-build that referenced this pull request Nov 20, 2019
as an workaround for Ruby bug 16331: https://bugs.ruby-lang.org/issues/16331

Due to this bug, build will fail with FreeBSD's make after rbenv#1368.
The bug is already fixed in MRI upstream but GNU make is still required
when building older releases of Ruby. Use GNU make rather than switching
make/gmake depending of Ruby version.

See also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=241633
@casperisfine
Copy link

As mentioned in #35 (comment), one potential concern is that shared library might be a bit slower.

Agreed. We recently disabled that flag and got a ~1.5% improvement on our response time.

Could this PR be reconsidered? Or at least could we make it easier to opt out? Because right now passing RUBY_CONFIGURE_OPTS=" --disable-shared " is ineffective, we actually have to patch all the definitions to opt-out.

@eregon
Copy link
Member

eregon commented Apr 22, 2021

Could this PR be reconsidered?

I think we should avoid changing the default back, as that would be an incompatible change for users relying on having the shared library (some gems need it).

Or at least could we make it easier to opt out?

That would be good. Could you make a PR to add that?

@casperisfine
Copy link

I think we should avoid changing the default back

Agreed.

Could you make a PR to add that?

I'll look into it.

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.

Build ruby with shared library
5 participants