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

New Approach to C/Java Native Extensions #186

Closed
jdantonio opened this issue Nov 15, 2014 · 18 comments
Closed

New Approach to C/Java Native Extensions #186

jdantonio opened this issue Nov 15, 2014 · 18 comments
Assignees
Labels
question An user question, does not change the library.
Milestone

Comments

@jdantonio
Copy link
Member

Now that we've had C/Java native extensions in the latest release for a few months, they have proven to be as big of a pain as I expected. For the next major release I suggest that we move all C and Java native code into the ruby-atomic gem, keep concurrent-ruby pure Ruby, have this gem detect the presence of ruby-atomic when installed, and use the optimized native code when present. Additionally, I suggest we use the current automated build process to build pre-compiled versions of ruby-atomic, but allow for compilation of extensions on install (which we currently do not do for concurrent-ruby).

A few other approaches have been suggested in other threads. Lets use this thread to discuss alternatives and make a decision.

The big problem posed by native extensions is obtaining a consistent build across all platforms. We've been able to manage this for many platforms by creating the pre-compiled builds. Things work as expected on Windows, JRuby, Rubinius, and standard Linux (Ubuntu and CentOS) installations. We have had problems on OS X (Issue #170) and Heroku (Issue #181). In both cases the pure-Ruby code is used even when the target platform supports compilations.

It is important to note that the issues #170 and #181 are the result of an intentional decision. Expecting a gem to compile on installation often causes problems because the necessary build tools may not be installed on the target system. This becomes an unrecoverable error that prevents installation of the gem. Even when the necessary build tools can be easily installed, many DevOps professionals prefer not to. Installing build tools on a production system is considered a security issue by many.

Although many applications will benefit by the performance improvements provided by the native code, many do not need them. Additionally, interpreters other than MRI and JRuby (such as Rubinius) cannot use the extensions anyway. Moving the native code into a different gem and making it optional seems like the most problem-free approach.

@jdantonio jdantonio added the question An user question, does not change the library. label Nov 15, 2014
@pitr-ch pitr-ch added this to the 0.8.0 Release milestone Nov 17, 2014
@pitr-ch
Copy link
Member

pitr-ch commented Nov 17, 2014

I have to admit I have no idea how big of pain it was so I may easily misjudge.

I am not sure I've understood the problem in #181? Heroku is always asking for pure Ruby?

I am assuming that we would like to ideally support:

  • precompiled versions
  • let gem to compile if there is no precompiled version for the platform and build tools are available
  • fallback to pure Ruby if all the above fails

Then we could enable gem compilation and find a way how to fallback gracefully to pure Ruby if compilation fails (that may not be possible based on quick googling).

Second option could be to provide two gems concurrent-ruby (only pure Ruby) and concurrent-ruby-native where the ..-native would provide precompiled versions or it would try to compile on installation. (It would be better to avoid monkey-patching but rather to build a helper object which would select implementations based on what is available on current platform. concurrent-ruby would provide just 'pure' and concurrent-ruby-native would provide others like C and Java for selection. Using one git repository with 2 gemspecs should help with maintainability. That would also allow to ask at runtime what implementation is used.)

@jdantonio
Copy link
Member Author

In #181, Heroku is not installing the pre-compiled gem. It is installing the pure-Ruby version. I haven't investigated yet, but there are two possible reasons that I can think of. 1) When Heroku pulls the gem from Rubygems it reports the "platform" as something Rubygems doesn't recognize so it defaults to the pure-Ruby version. 2) The user is caching the gem packages within the git repo (a very common practice with Rails) and the cached gem is the pure-Ruby one (possibly because development is being done on OS X). Either way, this isn't the behavior we desire. Heroku instanced have the build tools necessary for compiling the extensions so we want to make sure the compiled versions are installed.

I do not know if it is possible to have the base gem support both compile-at-install and a pure-Ruby fallback. The problem is that Rubygems performs the compilation as it installs the gems. The mechanism by which it does this was not designed to be optional. If the necessary build tools do not exist the Rubygems generates an error and fails the gem installation. I've read a few hacks online and experimented with a few of them during my initial exploration with native extensions, but I do not have confidence in them at this point. I will have to do a lot more testing. Even so, a working solution would still be a hack that circumvents the normal operation of Rubygems and we'll need to be careful.

@jdantonio
Copy link
Member Author

I had a conversation today with a developer from MongoDB who gave a tall on C-extensions. To her knowledge there is no way to optionally compile extensions at gem installation, which is what my research has indicated. She explained that at MongoDB they put all their C and Java extensions in separate, optional gems, and recommended that we do the same.

Presuming we take that approach, the remaining question is whether we create a new gem or use the existing ruby-atomic gem. I spoke to @headius briefly yesterday but haven't had a chance to ask him how he feels about us adding more code to ruby-atomic and using it that way.

@chrisseaton
Copy link
Member

I don't understand what the benefits of re-using ruby-atomic would be. Surely we just want concurrent-ruby, concurrent-ruby-native and concurrent-ruby-java?

@jdantonio
Copy link
Member Author

@chrisseaton No real advantage, but we have to do something with ruby-atomic. The original plan from @headius was to end-of-life that gem because the code is now in concurrent-ruby. It's been downloaded millions of times, however. What we don't know is whether the users of that gem will want to upgrade to concurrent-ruby (with vastly more code) or continue using the last release of ruby-atomic. If we decide to create a separate gem with the compiled atomic classes we may better serve the current users of ruby-atomic by keeping it alive and usable as a stand-alone gem. That's the only reason.

@chrisseaton
Copy link
Member

Could make ruby-atomic a thin layer over concurrent-ruby?

@jdantonio
Copy link
Member Author

I should also note that if we choose to keep ruby-atomic around as a stand-alone gem that integrates with concurrent-ruby, rather than create a new gem that is purely and extension of concurrent-ruby, we'll may end up adding some duplication. Specifically, we'll have to decide how to handle the pure-Ruby versions of the atomic classes.

When I get back home from RubyConf I can spike some of these ideas if that will help.

@jdantonio
Copy link
Member Author

I apologize for causing so much confusion. Let me start over. The question is this:

Now that we have merged all code from ruby-atomic into concurrent-ruby, what do we do about the 600 thousand people who have downloaded the latest version of ruby-atomic?

One option is to end-of-life ruby-atomic and force those users to upgrade to concurrent-ruby. Another option is to devise a solution that will keep ruby-atomic alive but fully integrated into concurrent-ruby.

The first option is best for the concurrent-ruby project but may disenfranchise the current users of ruby-atomic who will now need to install over 4600 LOC that they don't necessarily need. The second option is best for the current users of ruby-atomic but will cause additional complexity for this project. My original suggestion above was a suggestion for the second option.

If we (specifically @headius) decide to end-of-life ruby-atomic (the first option) then the plan for 8.0 should be fairly simple:

  • Create one more release of ruby-atomic that includes a deprecation warning (something like this PR
  • Move all our native C code into an optional extension gem called concurrent-ruby-ext that only works in conjunction with the main gem
  • Keep concurrent-ruby pure-Ruby (plus compiled Java) but have it automatically detect when concurrent-ruby-ext is installed and behave accordingly (similar to what we already do).

If we decide on the second option then we need to devise a plan similar to my original suggestion.

@headius @chrisseaton @mighe @pitr-ch @lucasallan Thoughts?

@jdantonio
Copy link
Member Author

It should also be noted that our pre-compiled Java gem package works flawlessly on all tested operating systems (Ubuntu and CentOS 32 and 64, Windows 32 and 64, Solaris, and OS X). This is to be expected thanks to the consistency of the JVM. Thus we would keep our compiled Java code in concurrent-ruby and only put compiled C code for MRI in concurrent-ruby-ext. (This is a change from what I originally stated but I've thought it through farther. I've updated my previous comment accordingly.)

At each release we would use our automated build process to create the following gem packages:

  • concurrent-ruby.gem -- pure Ruby code, all operating systems, all Ruby runtimes except JRuby
  • concurrent-ruby-java.gem -- pure Ruby + compiled Java code, all operating systems, JRuby only
  • concurrent-ruby-ext.gem -- uncompiled native C code, all operating systems, MRI only
  • concurrent-ruby-ext-x64-mingw32.gem -- compiled C code, Windows 64, MRI only
  • concurrent-ruby-ext-x86-mingw32.gem -- compiled C code, Windows 32, MRI only
  • concurrent-ruby-ext-x86_64-linux.gem -- compiled C code, Linux 64, MRI only
  • concurrent-ruby-ext-x86-linux.gem -- compiled C code, Linux 32, MRI only
  • concurrent-ruby-ext-x86-solaris-2.11.gem -- compiled C code, Solaris, MRI only

We would also consider a pre-compiled gem for MRI on OS X, assuming I can figure out the bug that caused me to yank concurrent-ruby- x86_64-darwin-13.gem from Rubygems.

@mighe
Copy link
Contributor

mighe commented Nov 19, 2014

atomic-ruby is a very very simple gem, it just provides an optimized and thread safe class and it does it really well. It won't probably require a lot of work in the future, since it is well focused, complete and stable. ( @headius made a great job!)
I think we could keep it alive (your second option) for that reason: at most we could add some kind of warning saying that we suggest to move to concurrent-ruby for a more complete library.

@mattwildig
Copy link

On Heroku, what I think is likely happening is that users add concurrent-ruby to their Gemfile and then (if they are on OSX) when they run bundle install Bundler is finding the pure Ruby version and adding that to the Gemfile.lock. Heroku will then try to install the pure Ruby version (Bundler will try to match the version exactly). So the gems don’t need to be cached in git – whatever version is installed on the platform that first runs bundle install will be the version used everywhere.

This is an known issue for people developing on Windows and trying to deploy to Heroku since precompiled Windows gems are more common than for other platforms.

@jdantonio
Copy link
Member Author

@mattwildig Thank you for that information! That makes a lot of sense. We use both Windows and Linux at work and I've run into that specific problem, it just never occurred to me that we would cause the same problem with our pre-compiled Linux gems.

Assuming that this is the case (and assuming my reasoning is correct), then we should be OK if we put all the C code in concurrent-ruby-ext.gem, release a precompiled Linux build, but do not release a pre-compiled OS X build. OS X would then cache the compilable version (concurrent-ruby-ext.gem), push that to Heroku, where it would then build.

Does my logic seem correct?

@mattwildig
Copy link

@jdantonio I’ve had a deeper look into this, and it turns out I was wrong and you should disregard my previous comment. Bundler will install a platform specific gem if one is available and the Gemfile.lock specifies a general one (although it won’t always do the other way round).

On Heroku the Linux precompiled gem is being installed, but is giving an error when the extension is required:

$ heroku run console
Running `console` attached to terminal... up, run.7945
irb(main):001:0> require 'concurrent'
Compiled extensions not installed, pure Ruby Atomic will be used.
=> true
irb(main):002:0> require 'concurrent_ruby_ext'
LoadError: libruby.so.1.9: cannot open shared object file: No such file or directory - /app/vendor/bundle/ruby/1.9.1/gems/concurrent-ruby-0.7.0-x86_64-linux/lib/concurrent_ruby_ext.so
    from (irb):2:in `require'
    from (irb):2
    from /app/bin/irb:18:in `<main>'

The problem seems to be to with trying to find libruby.so.1.9. The file in question is there, and looks like it has the appropriate permissions:

$ heroku run bash
Running `bash` attached to terminal... up, run.5865
~ $ cd vendor/bundle/ruby/1.9.1/gems/
~/vendor/bundle/ruby/1.9.1/gems $ ls -l
total 16
drwx------ 5 u9256 9256 4096 Nov 21 22:12 bcrypt-3.1.9
drwx------ 6 u9256 9256 4096 Nov 21 21:56 bundler-1.6.3
drwx------ 4 u9256 9256 4096 Nov 21 21:56 concurrent-ruby-0.7.0-x86_64-linux
drwx------ 7 u9256 9256 4096 Nov 21 21:56 rack-1.5.2
~/vendor/bundle/ruby/1.9.1/gems $ cd concurrent-ruby-0.7.0-x86_64-linux/lib/
~/vendor/bundle/ruby/1.9.1/gems/concurrent-ruby-0.7.0-x86_64-linux/lib $ ls -l
total 56
drwx------ 9 u9256 9256  4096 Nov 21 21:56 concurrent
-rw------- 1 u9256 9256  1328 Nov 21 21:56 concurrent.rb
-rwx------ 1 u9256 9256 37102 Nov 21 21:56 concurrent_ruby_ext.so
-rw------- 1 u9256 9256    21 Nov 21 21:56 concurrent_ruby.rb
-rw------- 1 u9256 9256   777 Nov 21 21:56 extension_helper.rb

But is trying to link against a non-existent file:

~/vendor/bundle/ruby/1.9.1/gems/concurrent-ruby-0.7.0-x86_64-linux/lib $ ldd concurrent_ruby_ext.so
    linux-vdso.so.1 =>  (0x00007fff4fbfe000)
    libruby.so.1.9 => not found
    libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fdda0475000)
    /lib64/ld-linux-x86-64.so.2 (0x00007fdda0a4a000)

This is at the edge of my knowledge of linking/loading, but the problem seems to be to do with Ruby on Heroku being statically compiled and the prebuilt gem expecting it to be dynamically compiled, with a libruby.so:

~/vendor/ruby-1.9.3 $ cd ~/vendor/ruby-1.9.3/
~/vendor/ruby-1.9.3 $ ls -l lib/
total 22892
-rw------- 1 u9256 9256 23429662 Nov 13 18:25 libruby-static.a
drwx------ 2 u9256 9256     4096 Nov 13 18:26 pkgconfig
drwx------ 6 u9256 9256     4096 Nov 13 18:26 ruby

@jdantonio
Copy link
Member Author

@mattwildig Awesome work! Thank you for investigating this! This information is extremely useful. We'll definitely take this into account when we decide how to handle extensions in future releases.

@jdantonio jdantonio self-assigned this Nov 25, 2014
@jdantonio
Copy link
Member Author

I have created a new concurrent-ruby-ext gem with the C extensions and create PR #197 which removes all C extensions from this gem. We need to decide if this is the approach we want to take. FWIW, my vote is "Yes").

@jdantonio
Copy link
Member Author

I've created a PR which should address this issue in the next major release along with a companion gem for the native C code that would compile on any platform with the necessary build tools (such as Heroku). Anyone interested in giving feedback, please do so on that PR. Thank you!

@jdantonio
Copy link
Member Author

PR #205 is the next iteration of a better approach.

@jdantonio
Copy link
Member Author

PR #205 addresses this. It is the basis for the 0.8.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question An user question, does not change the library.
Projects
None yet
Development

No branches or pull requests

5 participants