-
Notifications
You must be signed in to change notification settings - Fork 180
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
Change all T_DATA objects to typed data #349
Conversation
Classic T_DATA objects are deprecated since ruby-2.0. On the other hand TypedData objects can be checked more easy and allows some new features.
All T_DATA objects of ruby-pg are based on TypedData_Struct now. So we're no longer using deprecated non-typed objects.
This way the VALUE references may be relocated. Since self is always referenced indirectly, marking simple coders can be omitted entirely. This partly reverts 7c1756f which was introduced as a simple fix for GC.compact compatbility.
This way the VALUE references may be relocated. This partly reverts 7c1756f which was introduced as a simple fix for GC.compact compatbility.
spec/pg/gc_compact_spec.rb
Outdated
if GC.respond_to?(:compact) | ||
describe "GC.compact" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may be able to indent less using RSpec's built-in filtering.
if GC.respond_to?(:compact) | |
describe "GC.compact" do | |
describe "GC.compact", if: GC.respond_to?(:compact) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The filtering is a bit weird though as it still executes the block.
Probably fine here since all the logic is in before
/it
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
describe
blocks are still executed in case of if: false
. Only the execution of it
blocks is omitted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved all code into before, it and after blocks and made use of the :if filtering. Thanks for the hint!
@@ -0,0 +1,85 @@ | |||
# -*- rspec -*- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file seems to use tabs, but all other specs use the conventional 2-spaces indent, could you re-indent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, all ruby-pg source files use tabs indent (for some reason).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I mixed with FFI, sorry.
In the hope that the issue with TypedData inheritance is fixed there.
It is supported since ruby-2.1 and pg supports ruby-2.2+
@conninfo is not always defined in a describe context So define the constants in a before block and ensure the connection is closed. Moreover use rspec's :if filter to avoid indention "if: false" skips the execution of before, after and it blocks but the describe block is always executed.
The classic way to allocate T_DATA objects is deprecated since Ruby-2.0. The new way with rb_data_type_t structure has same advantages:
Currently TruffleRuby fails in CI, but the issue is fixed in oracle/truffleruby@f460ce7
This makes ruby-pg GC.compact friendly by reverting 7c1756f but using the compactation callback. This hopefully improves performance as described in #343.