Moved C extensions into a companion gem.

[jdantonio]

This is what this gem will look like if we move the C extensions into the concurrent-ruby-ext gem.

Packaging this repo

From this repo we would create two builds:

• Pure Ruby (for all platforms except JRuby)
• Ruby with compiled Java extensions (for JRuby)

We can retain the Java extensions in this rep because the pre-compiled Java build has worked flawlessly.

Packaging the extensions repo

The companion gem concurrent-ruby-ext will contain only pure-C code and a small amount of supporting Ruby code. We would still use the automated build process to create pre-compiled binaries. The difference is that the "base" gem version will compile on install if a pre-compiled package is not available. So we would push the following gems:

• concurrent-ruby-x.y.z.gem (compile C extension on install)
• concurrent-ruby-x.y.z-x86-linux.gem (Linux 32-bit)
• concurrent-ruby-x.y.z-x86_64-linux.gem (Linux 64-bit)
• concurrent-ruby-x.y.z-x86-mingw32.gem (Windows 32-bit)
• concurrent-ruby-x.y.z-x64-mingw32.gem (Windows 64-bit)

Usage

In your code make sure you require the extension gem before you require the core gem:

require 'concurrent_ext'
require 'concurrent'

[coveralls]

Coverage decreased (-0.12%) when pulling 77b70e1 on refactor/c-ext into df20680 on master.

[jdantonio]

I've prepared a PR that could be the final release of the atomic gem which was discussed back in May. I've also created an extension gem for the C native code that should resolve some of the issues we've been having with the C extensions not working as I had intended (#170, #181). It implements the plan proposed in #186 (specifically, this plan).

My recommendation is that we 1) end-of-life ruby-atomic, proceed with concurrent-ruby-ext as an optional extension gem for C native code on MRI, and 3) merge this PR.

Release 0.8.0 would then be the first release with the concurrent-ruby + concurrent-ruby-ext setup.

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

[SebastianEdwards]

What would be the recommended approach for gem authors using concurrent-ruby? Should we try to install/require the correct extensions depending on the end-users environment? Or use vanilla concurrent-ruby and leave it up to the end-user to install/require extensions for themselves?

[jdantonio]

@SebastianEdwards The extension gem would only apply to MRI users. All of the Java and Rbx optimizations would still be within the core concurrent-ruby gem and would be automatically enabled by the gem itself as it loads and detects the platform.

The only thing the C extensions provide are high-performance atomic objects (very fast locking). But the core gem has pure-Ruby versions of the same locking. Using the extensions gem would provide no additional functionality, just a potential performance boost for long running applications.

For gem authors using concurrent-ruby my recommendation would be to not worry about the extension gem at all. In your documentation you might have a small section to the effect of "Possible performance improvements for MRI users" and in there state something to the effect of "you may be able to get a slight performance boost when running on MRI by installing the extension gem..."

This is the reason we're being so careful to not force the C extensions on our users. The benefits they provide are really an edge case when extreme high performance is needed for long running applications on MRI.

Would that work for your needs?

[pitr-ch]

Hello @jdantonio,

Firstly thanks, this is lot of work and I can imagine not always fun. I have couple of notes:

• +1 for splitting to two gems
• On the other hand it might be better to keep one repository with multiple gem-specs. It'll help with maintainability.
• @iNecas had a good idea: we could keep concurrent-ruby providing pure-Ruby, precompiled java and C extensions and have concurrent-ruby-ext (or rather concurrent-ruby-compiled in this case) just to compile the extensions on gem installation (to help when prepackaged version is not available). Advantage is that for most of the users it'll just work.
• After we retire ruby-atomic (How do you plan to do it?) we should allow ruby-atomic users to load just what they need from concurrent-ruby, e.g. to require just concurrent-ruby/atomic.
• concurrent-ruby could warn the users when running on MRI without extensions (meaning downloading of precompiled version failed) to require concurrent-ruby-compiled to get the extensions. The warning can be on load time or when gem is installed. (I am in favor of load time.)
• I can imagine the order in require 'concurrent_ext'; require 'concurrent' being a real pain for users. Can we make the order irrelevant or fail when concurrent_ext is required second?

[schmurfy]

Splitting in two gems seems like the best path, here are some thoughts:

• I agree with @pitr-ch on having the two gems in the same repository, keeping two distinct repository in sync will require more work unless the c extension is mostly static and will not change often (which seems too be the case here).
• when you do a require concurrent_ext I think the simple path is to have the gem itself do the require 'concurrent' especially if this file is only requiring when using concurrent-ruby, if the extension is loaded after the main gem it should simply raise, plain and simple (see my point below about hiding important performance decisions from the users).
• atom-ruby merge: I already voiced my opinion about merging it in but with that done a user should still be able to use the atomic methods without using the full concurrent-ruby, but since most of the atomic gem is now inside concurrent-ruby and not in the extension gems if I get that right I am not sure what @pitr-ch propose is relevant. You would still need to have both the extension gem and concurrent-ruby to get what the atomic gem did before.
• I like the idea of showing a warning when adding the external gem could improve the performance (even if that's marginal), the worst example of what not to do was multi_xml I think which fallback on its internal full ruby parser when nothing better is available with no warning (now they added a warning I think), that's not quite comparable but a ghost to keep in check ^^

[pitr-ch]

I think we want the same thing: "user should still be able to use the atomic methods without using the full concurrent-ruby". concurrent-ruby/atomic should load just atomic parts even though concurrent-ruby and -ext would need to be installed.

I think that is a good approach in general to be able to load just parts of concurrent-ruby.

[schmurfy]

exactly :)
I hate to see big blobs of stuff like activesupport (this is probably the best example I have in store) where you could theoretically use the pieces you want (at least you can require them) but in practice everything is linked in it and requiring a small feature you need in activesupport often pulls everything else because of inter dependencies.

For me the end result is that I avoid everything using activesupport like the plague unless I really need to, I am confident concurrent-ruby won't take the same route and do things more intelligently.

[jdantonio]

All good suggestions. Some thoughts, in no particular order:

• I'll do another PR with two gemsec files in the one repo.
• I like the idea of concurrent-ruby-ext forcing the inclusion of concurrent-ruby so we'll do that as a dependency in the gemspec.
• Although it's not documented, I've always been very careful to no depend on transitive requires and also to provide "grouped" requires. require 'concurrent/atomic' should already work as suggested. Similarly, require 'concurrent/actor', require 'concurrent/channels', etc. should behave similarly. It should be possible to require a single abstraction such as require 'concurrent/promise' and get just what you need.
• I'll update the README to explain selective requires. I'll also add require 'concurrent/all' as an alias to require 'concurrent' so that we follow the Rails convention. This should help make it more obvious that we've divided the gem the way we have.
• Adding a warning about C extensions caused a lot of confusion in the 0.7.0 release, but I think the new configuration we're discussing should be less confusing. I'll add another warning when I create the next PR.
• I don't like the idea of having pre-compiled C extensions in the concurrent-ruby gem then using concurrent-ruby-ext only when the user needs to compile the extensions themselves. I think that might lead to more confusion. I think we're better off keeping the C code in only one gem, irrespective of pre-compilation.
• My suggestion for retiring ruby-atomic is in PR #5 of that repo
• Once concurrent-ruby-ext includes a dependency on concurrent-ruby then there should never be a need to explicitly require 'concurrent_ruby_ext'. Simply requiring the necessary utilities (require 'concurrent', require 'concurrent/atomic', require 'concurrent/promise', etc.) should wire things up appropriately, so I think the require-order issue will simply go away.

[pitr-ch]

Regarding the precompiled C-extensions on concurrent-ruby. I still think it may be better to do add precompiled C-extensions to concurrent-ruby, let me try to explain better why:

• User on JRuby or Rubinius: works as expected with concurrent-ruby in any case.
• User on MRI on supported platform gets precompiled C-extensions. It works for him without issues.
• User on MRI on unsupported platform does not get precompiled C-extensions. When the gem is used User gets warning, e.g. "We are sorry, this platform is not supported with precompiled C-extensions. Gem will work as expected using pure Ruby implementation. To get the performance boost provided by C-extensions please add 'concurrent-ruby-compiled' gem which will try to compile the extension while installing on your machine." User adds the gem and it works if it compiles.

In this way it will work for majority of the users without problems, if we don't add the precompiled C-extensions to concurrent-ruby, all MRI users (still majority) will have to add the concurrent-ruby-ext gem based on the warning they'll see.

[jdantonio]

@pitr-ch I understood what you meant regarding the C extensions. I'm just concerned that it will cause more confusion. The approach you suggest means that the C code will live in two places: the concurrent-ruby gem for many (most?) users (pre-compiled) and also in the concurrent-ruby-ext gem (uncompiled). This would be a very unconventional approach, however. It's a common practice to publish an extension gem with C code. Although what you suggest would be a little easier for most MRI users, I'm concerned that the unconventional approach might cause additional confusion.

This approach would also lead to a little extra internal complexity. We would have to include checks for cases where both a pre-compiled concurrent-ruby gem and the concurrent-ruby-ext gem (compiled at install) are both installed. We can't prevent a user from unnecessarily installing them both (especially when gem authors list us as a dependency).

With 0.7.0 I tried to be "clever" and automatically ease the burden of C extensions on our users. That didn't work out so well. Now I think that a conventional approach, though less elegant, might be less error-prone.

I'm certainly open to your suggestion if others think it's the more elegant approach so it's definitely worth more discussion.

[schmurfy]

Since the C extension provide a minor boost in performance I think that's fine to have it in its own gem, you can look at listen/guard for an example of standalone gems with optional c extensions gem.

[jdantonio]

PR #205 builds two gems from one repository. Please move further discussion to that PR.