Skip to content

Modernize usage of Ruby's C extension API#116

Merged
mudge merged 14 commits intomudge:mainfrom
casperisfine:typed-data
Nov 11, 2023
Merged

Modernize usage of Ruby's C extension API#116
mudge merged 14 commits intomudge:mainfrom
casperisfine:typed-data

Conversation

@casperisfine
Copy link

Use the TypedData API introduced in Ruby 1.9.2, instead of the old Data API that was deprecated in Ruby 2.3.

This allows to implement Write Barriers.

Write barrier protected objects are allowed to be promoted to the old generation, which means they only get marked on major GC, making minor GC faster.

The downside is that the RB_BJ_WRITE macro MUST be used to set references, otherwise the referenced object may be garbaged collected.

This also allow to implement GC compaction.

Copy link
Owner

@mudge mudge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for contributing this, I was unaware of the newer TypedData API.

There's no need to copy an already frozen string.
Rather than using a mix of, e.g. re2_matchdata* self and re2_matchdata
*self, always place the asterisk on the name rather than the type for
consistency with the rest of the code.
Define rb_gc_location as the identity function for Ruby < 2.7 so that
the code it generates is valid otherwise we get syntax errors like the
following:

    ../../../../ext/re2/re2.cc: In function 'void re2_matchdata_update_references(void*)':
    ../../../../ext/re2/re2.cc:142:46: error: expected primary-expression before ';' token
      142 |   self->regexp = rb_gc_location(self->regexp);
For Ruby versions < 2.7, there is no dcompact function for TypedData so
re2_matchdata_update_references and re2_scanner_update_references will
never be used resulting in a compiler warning. Avoid this by only
defining those functions when rb_gc_mark_movable is present.
@mudge
Copy link
Owner

mudge commented Nov 11, 2023

I also ran this through ruby_memcheck and no leaks were reported.

@mudge mudge merged commit 337a1f1 into mudge:main Nov 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants