Move more logic into Ruby code to use the gem in TruffleRuby

[eregon]

TruffleRuby, like other Ruby implementations, should support the FFI gem or equivalent API (so gems depending on ffi just work).
The FFI gem currently assumes that Ruby implementations other than MRI and Rubinius have their own equivalent FFI replacement.
This is not ideal, as it means those implementations (e.g., JRuby, TruffleRuby) need to maintain exact API compatibility and do not benefit from development here.

Instead, what I would like to do is to reuse the code of the FFI gem, but swap out the lower level parts.
While TruffleRuby does support C extensions, the Truffle framework already has its own optimized FFI called Truffle NFI. Truffle NFI uses libffi internally and it seems loading twice libffi would not be ideal.
A natural boundary for the "lower level parts" would be the C extension, and then reuse the Ruby code.

However, currently the FFI gem has quite a lot of C extension code (not counting libffi), and also exposes a fairly large API:

$ cd ext && cloc --exclude-dir=libffi . && cd ..
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
C                               25           1385           1948           5618
C/C++ Header                    24            303            616            900
make                             6             48             29            170
Ruby                             1             12              4             56
-------------------------------------------------------------------------------
SUM:                            56           1748           2597           6744
-------------------------------------------------------------------------------

$ cloc lib
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Ruby                            24            394           1335           1519
-------------------------------------------------------------------------------
SUM:                            24            394           1335           1519
-------------------------------------------------------------------------------

But a fair part of the C extension code doesn't seem to need to be written in C.
In fact, I started rewriting parts of the C extension to Ruby, and it seems to work well so far.

So I would like to know if PRs moving C extension logic to Ruby would be welcome.
I think it would be good as it would make the gem more maintainable by having clearer and more expressive code than is possible with the C extension API.
Of course, performance is a concern so translating performance-critical parts needs to be evaluated.

I will start with a simple PR moving a class from C to Ruby.

[larskanis]

@tduehr do you have any objections? I'm fine with this direction.

@eregon there's already a partial ffi implementation on top of fiddle, which might be helpful: https://github.com/unak/fiddley

[larskanis]

@eregon just to make sure: there's still a bunch of compatibility code for older ruby versions in ffi. Don't bother about it. I just didn't have the time to remove it.

[eregon]

@eregon there's already a partial ffi implementation on top of fiddle, which might be helpful: https://github.com/unak/fiddley

Interesting, thanks for the link. I know from trying that OptCarrot only uses a small subset of FFI (basically FFI::Library, attach_function, and FFI::Struct) and that repo seems to indeed only have a small subset of the FFI API currently.

@eregon just to make sure: there's still a bunch of compatibility code for older ruby versions in ffi. Don't bother about it. I just didn't have the time to remove it.

What's the minimum required Ruby version? Ruby 2.3 as the .travis.yml?

[larskanis]

What's the minimum required Ruby version? Ruby 2.3 as the .travis.yml?

I think so. So far there was no reason to deny installs on older ruby versions, so that the gemspec still states >= 1.9.

[headius]

This would be a good opportunity to unify the copy of the Ruby files that lives in JRuby as well, since that and the FFI gem tend to drift apart over time. @tduehr and others have helped us align bits and pieces but we should try to source them all from a single location.

Moving stuff into Ruby is fine if it's not on a critical path and doesn't severely impact performance on MRI.

[tduehr]

How much will have to move to Ruby for it to run in TruffleRuby? It can’t all go.

There’s not a whole lot of code that can move out of Java and C into Ruby. I’d prefer to keep back of the house code in C and Java for performance reasons.

I think we already have performance and runtime optimization issues (just based on the fact this is a bridge layer between two different runtimes). We need to keep an eye on how often we cross between the two (or three in Java). The more we stay out of Ruby, I expect Java (and GraalVM) will have a better time with JIT and stuff. Same for the MRI extension and llvm.

Is there a way to package the JRuby implementation of FFI as a gem on its own? Is TruffleRuby’s internal API close enough to JRuby for that to work?

[tduehr]

Please tell me Fiddley will run in JRuby... It might be fun to run FFI implemented on Fiddle implemented on FFI. The

Wrt #661, I seem to remember this being one of the darker corners of the JRuby implementation. I’ll have to take a deeper look at the two to make sure.

[tduehr]

Now that I’ve read all of the OP, seems like there’s two things to be done here:

1. Review the code for stuff that can and should be ported over to Ruby.
2. Create a new gem, ffi-core, to house the pure Ruby parts of FFI.

For 1, we really need a benchmark suite to validate changes.

For 2, we “just” have to figure out testing across multiple disparate codebases.

[eregon]

@tduehr

How much will have to move to Ruby for it to run in TruffleRuby? It can’t all go.

I know that I have large parts of FFI running in TruffleRuby with only pure Ruby code and Truffle NFI, using the FFI C extension code translated to Ruby.
I could just keep that in TruffleRuby, but it seems nicer to move and share the code here, as long as performance is maintained.

There’s not a whole lot of code that can move out of Java and C into Ruby. I’d prefer to keep back of the house code in C and Java for performance reasons.

I had a look at various parts of the C extension, there is actually a lot of Ruby-like logic there which IMHO has no benefit to be written in C.

The more we stay out of Ruby, I expect Java (and GraalVM) will have a better time with JIT and stuff. Same for the MRI extension and llvm.

I would think the opposite, all Ruby JITs can optimize Ruby code, but crossing the C or Java extension is an extra barrier for most of them.
But instead of trying to find a general rule, I'd much prefer to just run benchmarks and show there is no significant regression.

Please tell me Fiddle will run in JRuby... It might be fun to run FFI implemented on Fiddle implemented on FFI.

TruffleRuby does run Fiddle, but I'm not sure whether Fiddle is enough to support the entire functionality of the FFI gem. If we did, then the FFI gem would be pure Ruby though.

Now that I’ve read all of the OP, seems like there’s two things to be done here:

1. Review the code for stuff that can and should be ported over to Ruby.

Yes, please review #661. I think there are only advantages for that one, so it's an easy starting point.

For 1, we really need a benchmark suite to validate changes.

How about running https://github.com/ffi/ffi/tree/master/bench ?

2. Create a new gem, ffi-core, to house the pure Ruby parts of FFI.
   For 2, we “just” have to figure out testing across multiple disparate codebases.

I don't think it's necessary to split the FFI gem. We can just skip compiling the C extension on non-MRI non-Rubinius as it's done currently.

[eregon]

@tduehr

Is TruffleRuby’s internal API close enough to JRuby for that to work?

No, the FFI Java code in JRuby depends on too many JRuby-internal types: https://github.com/jruby/jruby/blob/562e1f911f8d386ef5b021c7c538f74c5e557f01/core/src/main/java/org/jruby/ext/ffi/FFI.java

The Ruby FFI code in JRuby seems to be similar to an older version of this gem, is that right @headius?

[tduehr]

@eregon most of it should be the same but it does get out of sync here and there. You have to look at both really...

Please tell me Fiddle will run in JRuby... It might be fun to run FFI implemented on Fiddle implemented on FFI.

TruffleRuby does run Fiddle, but I'm not sure whether Fiddle is enough to support the entire functionality of the FFI gem. If we did, then the FFI gem would be pure Ruby though.

I'd meant Fiddley ... The fiddle implementation in JRuby uses the FFI gem, IIRC. I want to say you could implement the Ruby API of FFI using Fiddle if you really wanted but you'd likely wake The Great Old Ones in the process.

I don't think it's necessary to split the FFI gem. We can just skip compiling the C extension on non-MRI non-Rubinius as it's done currently.

Can we pretend this is what I meant? It's way easier... Though, the testing issues still exist. We could probably work that out with a little travis kung fu.

How about running https://github.com/ffi/ffi/tree/master/bench ?

Last I knew, that benchmark suite didn't work.

[headius]

I would think the opposite, all Ruby JITs can optimize Ruby code, but crossing the C or Java extension is an extra barrier for most of them.

Calling Java-based extensions from Ruby incurs zero penalty on JRuby. They will usually match arity and inline very well with invokedynamic.

Calling arbitrary Java libs (not used here) also inlines well with indy except in arity-mismatched or interface coersion situations (but that's just missing opto).

I believe MRI has no big penalty calling into C exts either, except for more frequent need to box arguments into an array.

[headius]

@eregon most of it should be the same but it does get out of sync here and there. You have to look at both really...

And I'm sure there are fixes and features in each that don't exist in the other. We really need to get as much of that Ruby codes synced up and into the gem as possible.

[eregon]

I believe MRI has no big penalty calling into C exts either, except for more frequent need to box arguments into an array.

At least in MJIT, Ruby->C doesn't inline now. What I meant was the opposite direction, C->Ruby with rb_funcall() or similar. That's uncached and non-inlined calls in MRI, so I would expect some overhead. Similar for accessing instance variables from C code. IMHO, that's the kind of code that is better written in Ruby than in C, and is also typically more readable.
Anyway, I think we agree that moving more to Ruby and sharing code is good, while watching critical path performance with benchmarks.

And I'm sure there are fixes and features in each that don't exist in the other. We really need to get as much of that Ruby codes synced up and into the gem as possible.

Yes, which is one of the main motivations as stated in the first post.
I think it would be great if JRuby would use the Ruby files of the FFI gem, and then instead of using the FFI C extension, use the bundled JRuby extension in JRuby (or move it here).
What do you think @headius?

The big picture is a FFI gem with most of the API and logic in pure Ruby, and then small backends for what cannot be expressed or is not efficient enough in Ruby. Then we could have a C ext + libffi, JRuby ext, Truffle NFI and Fiddle backends while exposing the exact same public API. Some of these backends would live inside this repo and some outside, but this is not a big problem as the smaller backend API would be small and stable.

[tduehr]

Looks like the benchmark code is expecting the old dl library. That's going to need to be ported to fiddle to get those comparisons.

[headius]

I think we agree that moving more to Ruby and sharing code is good, while watching critical path performance with benchmarks.

Moving non-critical code to Ruby is a good goal. I do not know enough about the C ext to know how much of it would be non-critical.

[headius]

I think it would be great if JRuby would use the Ruby files of the FFI gem, and then instead of using the FFI C extension, use the bundled JRuby extension in JRuby (or move it here).

Yeah that's what I've always wanted. The files in JRuby were a copy mostly because it was cumbersome to install them at build time and we needed FFI for core JRuby features. Then they just started to drift. @tduehr helped us sync some of those files, but we have never quite gotten to the point of sourcing the Ruby code directly from the gem.

[tduehr]

Could you use the gem as a submodule somehow?

[headius]

@tduehr I am actually fond of the splitting of the gem, since it would mean we could release an ffi-ext for JRuby and let RG sort out platforms and such. The FFI gem itself would just have a dependency on some FFI backend, which would be exts on most impls or based on Fiddle perhaps.

In any case I think the plan for JRuby would be to make FFI a default gem, so at build time all the .rb files get stuffed into stdlib in the same place they exist now. JRuby needs FFI at boot for some features (though most native calls go through our JNR libraries) so it's better if it gets fully installed into stdlib and can run without RubyGems loading.

This exercise is basically formalizing the API between the backend (ext) and frontend (ruby) so we can all use the same frontend with our own backends. Moving more into Ruby reduces the surface area of that API.

[tduehr]

I'm okay with that.

Running benchmarks now. Looks like the C version may have a slight edge on Ruby but that's over 10m trials.

Anyone know of an equivalent of travis for these benchmark runs?

[tduehr]

Best I have so far is benchmark-ips https://github.com/evanphx/benchmark-ips

We can set something up to run this in travis across several rubies fairly easily.

[tduehr]

They're pretty close:

https://gist.github.com/tduehr/1237a64b57800d11029bc12fb32981bb

[tduehr]

Here's a quick and dirty PoC rake task using the benchmark-ips framework. This is just a dummy; the numbers aren't real.

https://gist.github.com/tduehr/ca21fb7c9ec401b4c0bd59d981b6fed9

[eregon]

@tduehr Did you find a way to have a summary showing how much speedup/slowdown on each benchmark, and a global average?

https://github.com/benchmark-driver/benchmark-driver might be helpful.

[tduehr]

I hadn’t run across that one. It’s closer to what we need but still not quite there yet.

[tduehr]

Looks like we’ll have to use that and built something like an A/B testing suite around it.

[eregon]

@tduehr benchmark-driver is also used for MRI benchmarks, which I would think have very similar needs (comparing 2 MRI builds instead of 2 FFI implementations, but we can express the later as the former). I think it would be worth to raise an issue explaining what you'd want as there might already be a way to do it.

[larskanis]

@eregon I opened PR #663 and #664 that should go into your direction.

@headius In the past I didn't pay JRuby-ffi much attention, but due to this talk I now inspected the JRuby implementation of ffi and found that the ruby part is mostly an old copy of this gem. And the java part looks like an ordinary JRuby extension.

OMHO it would make sense to add the JRuby extension to this repository and align it with the ruby part. This makes it similar to how we work on Nokogiri. The main difference is that FFI is required for core JRuby features. I don't like the idea of splitting ffi into several gems, since that OMHO just makes things more complicated.

In the second step the JRuby embedded ffi could be replaced by the content of this repo. I'm not sure which method fits well to JRuby. Maybe a rake tasks which updates the files from this repo to JRuby before the release? Or adding this repo as a git submodule? Or make the ffi gem a default gem?

I think the most important task in order to align the different FFI implementations is to use common specs. So we need to add JRuby and Truffleruby to our CI. I opened PR #664 therefore.

[headius]

Or make the ffi gem a default gem?

This is the best option. FFI would still be upgradeable through the default gem logic but always available for JRuby needs with or without RubyGems loaded.

[larskanis]

On the join-ruby branch I added the JRuby extension code from JRubys ext directory. I moved the extension code to another namespace, bundled external jar dependencies and partly adjusted the code to fit to @eregon 's changes. It doesn't properly start rake test right now, due to missing features which are required while loading some specs, but it already passes most of the specs, when started separately per rspec.

@headius Could you imagine to unify the ffi development that way? And are you willing to maintain the FFI code of JRuby within this repository in the future and include the ffi gem instead? My Java knowledge is very limited, so that I'm unable to do any deeper changes to it (in particular to the JIT parts).

[eregon]

I'm now further on and realize that some parts such as, e.g. FFI::Struct#[] seem difficult to move to Ruby without impacting performance on MRI. The C code in this area is also fairly coupled and complex, making it hard to move things progressively.

So for the moment I'll continue on a pure-Ruby implementation in TruffleRuby, and then see how that can be integrated back here. I think the whole FFI::Type hierarchy can be mostly Ruby for instance, but it's likely a rather big change.

My current progress is on https://github.com/eregon/ffi/commits/mostly-ruby-ffi, which @larskanis already found :)
In any case, I think it would be useful to merge the changes I have so far, as it would make a smaller diff, share more code and ease further refactorings to Ruby and integration of the JRuby backend. And I think the changes improve readability and therefore also maintainability.
I have to say I'm a bit discouraged by #661 taking so long to merge when it seems such an obvious gain to me. cc @tduehr

[tduehr]

I was going to get a benchmark set up before merging it since eyeballing the existing benchmarks looked like #661 might be slower. Benchmark suite proved to be more difficult than i'd expected and I still haven't gotten it figured out.

[eregon]

Right, that would be useful. I think #661 shouldn't affect performance significantly (if even identity functions in Ruby are slow, that's a bigger problem).
I think it would be fine to just run one or two benchmarks we know use the changed code, if running the whole suite is complicated. E.g., calls not using DataConverter should be unaffected, calls using it should be roughly as fast.

@tduehr Could you commit your fixes to the benchmark harness?
For me, it currently fails with:

.../gems/ffi-1.10.0/lib/ffi/library.rb:145:in `block in ffi_lib': Could not open library '.../ffi/build/libtest.so': .../ffi/build/libtest.so: cannot open shared object file: No such file or directory (LoadError)
	from .../gems/ffi-1.10.0/lib/ffi/library.rb:99:in `map'
	from .../gems/ffi-1.10.0/lib/ffi/library.rb:99:in `ffi_lib'
	from bench/bench_IIIrV.rb:5:in `<module:LibTest>'
	from bench/bench_IIIrV.rb:3:in `<main>'

And if you did the conversion to benchmark/ips or benchmark-driver, could you commit that too? If not, the existing benchmark stdlib is probably fine too.
Then I could run a couple benchmarks easily.

[tduehr]

working benchmarks in #670 ips based benchmarks to follow shortly. Is there a statistician in the house?

[eregon]

@tduehr Thanks, I also needed #676 to run benchmarks.

Is there a statistician in the house?

Any question regarding statistics?

Moving to benchmark-ips seems better anyway, as the number of iterations no longer needs to be hardcoded.

[eregon]

I now completed a pure-Ruby + Truffle NFI implementation of FFI in TruffleRuby (since 1.0.0-rc16), passing the vast majority of FFI specs: https://github.com/oracle/truffleruby/tree/master/lib/truffle/truffle/ffi_backend

To give some numbers, this Ruby implementation of FFI is only 1097 SLOC while currently the C extension is 5900 SLOC (as counted by cloc), illustrating C code is quite a bit more verbose.

In my FFI implementation, everything is in Ruby except a small libffi wrapper (called Truffle NFI), which only needs a few primitives to, e.g., call a native function, create a handler for a Ruby Proc, accessing errno, and accessing native memory with the various C types (e.g., with FFI::Pointer).
The Ruby backend replaces the C extension, using the same API.

It would be nice to share more of this code with the FFI gem, however some parts are likely performance-sensitive. For instance, coercion from native to Ruby and back for accessing native structs is done using Ruby methods on the type (FFI::Type#get_at/put_at) rather than in native code, by reusing FFI::Pointer methods.

[eregon]

@larskanis Do you think it would be possible to make a release of FFI with latest master soon?

I'd like to import the latest changes on master, and there have been 80 commits since the last 1.10.0 release, including a fair bit of spec cleanups and the PRs I did for this issue.
This would also make it easier for me to add and upstream FFI specs, and keep them synchronized with the version in TruffleRuby.

[larskanis]

@eregon Yes, I'll release a new version today.

[eregon]

@larskanis Awesome, thanks!

[eregon]

jruby/jruby#5947 is related and also details how TruffleRuby currently reuses FFI files.

[eregon]

I'm updating the TruffleRuby implementation of FFI to use the latest 1.12.2 release.
As part of this I will try to upstream the remaining changes we have compared to master.
That will also enable adding TruffleRuby in FFI's CI.

[eregon]

@larskanis @tduehr Could you consider giving me write access to this repository so I can add new specs, etc more efficiently?

I'm the second contributor by #commits to this repository in the last couple years, I fixed the benchmark harness, the test compilation harness, updated the README, etc: https://github.com/ffi/ffi/pulls?q=is%3Apr+is%3Aclosed+author%3Aeregon

[headius]

FYI jruby/jruby#5947 has been resolved in JRuby. All files have been updated and all FFI specs pass.