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

Add support for FFI while leaving legacy implementations intact #11483

Closed
wants to merge 58 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
394321f
branch to support FFI
JasonLunn Oct 31, 2022
11fcd23
restore protobuf.rb
JasonLunn Oct 31, 2022
09a7114
native implementation
JasonLunn Oct 31, 2022
1e50c8b
restore protobuf.rb
JasonLunn Oct 31, 2022
6cc806c
Refactor `protobuf.rb` to separate native and ffi implementations.
JasonLunn Oct 31, 2022
dfc7dbb
Fix spelling `Unsupport` -> `Unsupported`
JasonLunn Jan 6, 2023
fb395f8
Fix exception raising by properly instantiating the exception.
JasonLunn Jan 6, 2023
d9767ca
Add check on `PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION` environment varia…
JasonLunn Jan 6, 2023
3792576
Add FFI implementation
JasonLunn Jan 6, 2023
589629d
Add ffi flags
mkruskal-google Jan 10, 2023
2ec3aa3
Refactor convert.c to separate CRuby-specific glue that has dependenc…
JasonLunn Jan 11, 2023
a5ff141
Fix copy/paste casting bug.
JasonLunn Jan 11, 2023
cec7850
Refactor message.c to separate CRuby-specific glue that has dependenc…
JasonLunn Jan 11, 2023
4734f34
Have shared_convert methods invoke shared_message methods directly.
JasonLunn Jan 11, 2023
55cd7fe
branch to support FFI
JasonLunn Oct 31, 2022
2f3838b
restore protobuf.rb
JasonLunn Oct 31, 2022
c8656c4
native implementation
JasonLunn Oct 31, 2022
1c19455
restore protobuf.rb
JasonLunn Oct 31, 2022
f461e72
Refactor `protobuf.rb` to separate native and ffi implementations.
JasonLunn Oct 31, 2022
1b1e64e
Fix spelling `Unsupport` -> `Unsupported`
JasonLunn Jan 6, 2023
4cbcd56
Fix exception raising by properly instantiating the exception.
JasonLunn Jan 6, 2023
5d71437
Stop ignoring shared source files; ignore FFI Compiler output directo…
JasonLunn Jul 6, 2023
b402119
Update function names and arguments.
JasonLunn Jul 6, 2023
f7083f1
Remove references to `PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION`.
JasonLunn Jul 6, 2023
039af51
Relocate shared files.
JasonLunn Jul 6, 2023
8607cda
Temporarily update visibility, pending update to upb repo.
JasonLunn Jul 6, 2023
1a716c0
Create glue.
JasonLunn Jul 6, 2023
e7240f8
Fix unit tests.
JasonLunn Jul 7, 2023
8f95755
Rebase
JasonLunn Oct 31, 2022
1dff209
restore protobuf.rb
JasonLunn Oct 31, 2022
f4c08b2
Rebase
JasonLunn Oct 31, 2022
e32280e
restore protobuf.rb
JasonLunn Oct 31, 2022
536f9b3
Rebase
JasonLunn Oct 31, 2022
c665a79
Fix spelling `Unsupport` -> `Unsupported`
JasonLunn Jan 6, 2023
78a2928
Fix exception raising by properly instantiating the exception.
JasonLunn Jan 6, 2023
a7aa7aa
Rebase
JasonLunn Jan 6, 2023
bcc6f6c
Rebase
JasonLunn Jan 11, 2023
8336d0a
Refactor message.c to separate CRuby-specific glue that has dependenc…
JasonLunn Jan 11, 2023
88f1506
Have shared_convert methods invoke shared_message methods directly.
JasonLunn Jan 11, 2023
3089818
Fix merge damage
JasonLunn Jul 7, 2023
95c4535
Call `upb_Arena_Malloc()` directly.
JasonLunn Jul 7, 2023
0286c71
Address PR feedback.
JasonLunn Jul 7, 2023
2fd1eac
Cleanup debug print.
JasonLunn Jul 7, 2023
3d1d1fe
Revert changes to ruby-upb.h.
JasonLunn Jul 10, 2023
0d575d5
Merge remote-tracking branch 'upstream/main' into simultaneous_ffi
JasonLunn Jul 10, 2023
8920f9d
Update to the new Ruby-native ObjectCache.
JasonLunn Jul 10, 2023
72d6ee8
Allow fallback to native CRuby implementation when FFI doesn't load.
JasonLunn Jul 10, 2023
a3eb317
Introduce a new test that the expected implementation is used.
JasonLunn Jul 12, 2023
bc87afb
Merge remote-tracking branch 'upstream/main' into simultaneous_ffi
JasonLunn Jul 12, 2023
ca80881
Merge branch 'main' into simultaneous_ffi
JasonLunn Jul 13, 2023
2c8d0ef
Use the FFI-fix branch of rules_ruby.
JasonLunn Jul 14, 2023
2a1cf7c
Use backwards compatible range syntax.
JasonLunn Jul 14, 2023
e28e256
Move most calls to `attach_function` out of `ffi.rb`.
JasonLunn Jul 15, 2023
55f2884
Breakout conformance tests by platform and whether FFI is enabled.
JasonLunn Jul 15, 2023
03a32da
Merge remote-tracking branch 'upstream/main' into simultaneous_ffi
JasonLunn Jul 15, 2023
472b653
Remove workaround for access to UPI API functions.
JasonLunn Jul 16, 2023
af40b04
Default JRuby to autodetecting FFI but make CRuby request it explicitly.
JasonLunn Jul 17, 2023
839a3d7
Make all interpreters opt-in for FFI.
JasonLunn Jul 18, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Make all interpreters opt-in for FFI.
Rename `backend.rb` to `implementation.rb`.
  • Loading branch information
JasonLunn committed Jul 18, 2023
commit 839a3d77e35488ffe32f25d53978f7fa1d6d291d
6 changes: 6 additions & 0 deletions ruby/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ conformance_test(

conformance_test(
name = "conformance_test_ffi",
env = {
"PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION": "ffi",
},
failure_list = "//conformance:failure_list_ruby.txt",
target_compatible_with = select({
":ruby_ffi": [],
Expand All @@ -213,6 +216,9 @@ conformance_test(

conformance_test(
name = "conformance_test_jruby_ffi",
env = {
"PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION": "ffi",
},
failure_list = "//conformance:failure_list_jruby_ffi.txt",
target_compatible_with = select({
":jruby_ffi": [],
Expand Down
12 changes: 8 additions & 4 deletions ruby/Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@ require "rubygems"
require "rubygems/package_task"
require "rake/extensiontask" unless RUBY_PLATFORM == "java"
require "rake/testtask"

# FFI and FFI-Compiler are developer dependencies everywhere but only
# dependencies for JRuby right now.
begin
require 'ffi'
require "ffi-compiler/compile_task"
USE_FFI = true
FFI_AVAILABLE = true
rescue LoadError
USE_FFI = false
warn "LoadError in Rakefile while loading FFI: #{$!}. FFI-related tasks unavailable. To enable, install `ffi` and `ffi-compiler` gems."
FFI_AVAILABLE = false
end

spec = Gem::Specification.load("google-protobuf.gemspec")
Expand Down Expand Up @@ -54,7 +58,7 @@ end

genproto_output = []

if USE_FFI
if FFI_AVAILABLE
common_excluded_source = [
:convert, :defs, :map, :message, :protobuf, :repeated_field, :wrap_memcpy
]
Expand Down Expand Up @@ -233,7 +237,7 @@ else
end
end

task :compile => ["ffi-compiler:default"] if USE_FFI
task :compile => ["ffi-compiler:default"] if FFI_AVAILABLE

task :genproto => genproto_output

Expand Down
26 changes: 5 additions & 21 deletions ruby/lib/google/protobuf.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,26 +41,12 @@ class ParseError < Error; end
class TypeError < ::TypeError; end

PREFER_FFI = case ENV['PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION']
when nil, ""
if RUBY_PLATFORM == "java"
# When unspecified, PREFER_FFI is autodetected based on
# available gems for JRuby. For CRuby, users must opt-in.
begin
require 'ffi'
require 'ffi-compiler/loader'
true
rescue LoadError
false
end
else
false
end
when nil, "", /^native$/i
false
when /^ffi$/i
true
when /^native$/i
false
else
warn "Unexpected value `#{ENV['PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION']}` for environment variable `PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION`. Should be either \"FFI\", \"NATIVE\". Omit for autodetection."
warn "Unexpected value `#{ENV['PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION']}` for environment variable `PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION`. Should be either \"FFI\", \"NATIVE\"."
false
end

Expand All @@ -85,10 +71,8 @@ def self.decode_json(klass, json, options = {})
require 'google/protobuf_ffi'
:FFI
rescue LoadError
if $DEBUG
warn "Caught exception `#{$!.message}` while loading FFI implementation of google/protobuf."
warn "Falling back to native implementation."
end
warn "Caught exception `#{$!.message}` while loading FFI implementation of google/protobuf."
warn "Falling back to native implementation."
require 'google/protobuf_native'
:NATIVE
end
Expand Down
4 changes: 2 additions & 2 deletions ruby/tests/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ filegroup(
)

ruby_test(
name = "backend",
srcs = ["backend.rb"],
name = "implementation",
srcs = ["implementation.rb"],
deps = [
"//ruby:protobuf",
"@protobuf_bundle//:test-unit",
Expand Down
11 changes: 0 additions & 11 deletions ruby/tests/backend.rb

This file was deleted.

37 changes: 37 additions & 0 deletions ruby/tests/implementation.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
require 'google/protobuf'
require 'test/unit'

class BackendTest < Test::Unit::TestCase
# Verifies the implementation of Protobuf is the preferred one.
# See protobuf.rb for the logic that defines PREFER_FFI.
def test_prefer_ffi_aligns_with_implementation
expected = Google::Protobuf::PREFER_FFI ? :FFI : :NATIVE
assert_equal expected, Google::Protobuf::IMPLEMENTATION
end

def test_prefer_ffi
unless ENV['PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION'] =~ /ffi/i
omit"FFI implementation requires environment variable PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION=FFI to activate."
end
assert_equal true, Google::Protobuf::PREFER_FFI
end
def test_ffi_implementation
unless ENV['PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION'] =~ /ffi/i
omit "FFI implementation requires environment variable PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION=FFI to activate."
end
assert_equal :FFI, Google::Protobuf::IMPLEMENTATION
end

def test_prefer_native
if ENV.include?('PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION') and ENV['PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION'] !~ /native/i
omit"Native implementation requires omitting environment variable PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION or setting it to `NATIVE` to activate."
end
assert_equal false, Google::Protobuf::PREFER_FFI
end
def test_native_implementation
if ENV.include?('PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION') and ENV['PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION'] !~ /native/i
omit"Native implementation requires omitting environment variable PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION or setting it to `NATIVE` to activate."
end
assert_equal :NATIVE, Google::Protobuf::IMPLEMENTATION
end
end
Loading