Skip to content

Optimize allocations with embedded TypedData #852

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.

Sign up for GitHub

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

Draft
amomchilov wants to merge 3 commits into
base: Alex/Ruby-owned-graph-alloc
Choose a base branch
from
Draft

Optimize allocations with embedded TypedData #852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
feaf794
Extract out `ruby_compat.h`
amomchilov Jun 9, 2026
fc86de0
Use embedded TypedData allocations
amomchilov Jun 9, 2026
d14b96b
Use `RUBY_DEFAULT_FREE` in embedded case
amomchilov Jun 10, 2026
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
6 changes: 6 additions & 0 deletions ext/rubydex/extconf.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@
append_cflags("-Werror=unused-but-set-variable")
append_cflags("-Werror=implicit-function-declaration")

# Compiles a minimal program to test if the `RUBY_TYPED_EMBEDDABLE` C constant exists ( Ruby 3.3 or newer).

@amomchilov amomchilov Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is verbose, but there's so much magic going on here that it's worth documenting.

For one, explicitly mentioning the constant name means that when you search for where HAVE_CONST_RUBY_TYPED_EMBEDDABLE comes from, you'll find this.

# * If it exists, the generated makefile will set a preprocessor macro: `#define HAVE_CONST_RUBY_TYPED_EMBEDDABLE 1`
# * else: `#define HAVE_CONST_RUBY_TYPED_EMBEDDABLE 0`
# See https://ruby-doc.org/3.3.4/stdlibs/mkmf/MakeMakefile.html#method-i-have_const
have_const("RUBY_TYPED_EMBEDDABLE", "ruby.h")

# There's an error on Windows with function pointer types not matching. This has been fixed and backported in Ruby, but
# it seems that RubyInstaller sometimes picks an older patch version on CI and it breaks compilation. This isn't
# actually a problem, so we're ignoring it temporarily only on Windows
Expand Down
8 changes: 7 additions & 1 deletion ext/rubydex/graph.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "location.h"
#include "reference.h"
#include "ruby/internal/globals.h"
#include "ruby_compat.h"
#include "rustbindings.h"
#include "utils.h"

Expand Down Expand Up @@ -44,8 +45,13 @@ static void graph_free(void *ptr) {
// let the Rust side drop the Graph struct internally.
rdx_graph_drop(ptr);

#ifdef HAVE_RUBY_TYPED_EMBEDDABLE
// The storage is embedded in the TypedData Ruby object itself.
// Don't free `ptr`, because the GC will do that for us.

@st0012 st0012 Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: just to make it clearer which GC it means (I know Rust doesn't have GC, but maybe now everyone reading this knows)

Suggested change
// Don't free `ptr`, because the GC will do that for us.
// Don't free `ptr`, because the Ruby GC will do that for us.

#else
// Free the TypeData Ruby object itself
xfree(ptr);
#endif
}
}

Expand All @@ -59,7 +65,7 @@ const rb_data_type_t graph_type = {
},
.parent = NULL,
.data = NULL,
.flags = RUBY_TYPED_FREE_IMMEDIATELY,
.flags = RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_EMBEDDABLE,
};

// Custom allocator for the Graph class.
Expand Down
10 changes: 8 additions & 2 deletions ext/rubydex/handle.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#ifndef RUBYDEX_HANDLE_H
#define RUBYDEX_HANDLE_H

#include "ruby.h"
#include "ruby_compat.h"

typedef struct {
VALUE graph_obj; // Ruby Graph object to keep it alive
Expand All @@ -15,23 +15,29 @@ static void handle_mark(void *ptr) {
}
}

#ifndef HAVE_RUBY_TYPED_EMBEDDABLE
static void handle_free(void *ptr) {
if (ptr) {
xfree(ptr);
}
}
#endif

static const rb_data_type_t handle_type = {
.wrap_struct_name = "RubydexHandle",
.function = {
.dmark = handle_mark,
#ifdef HAVE_RUBY_TYPED_EMBEDDABLE
.dfree = RUBY_DEFAULT_FREE,
#else
.dfree = handle_free,
#endif
.dsize = NULL,
.dcompact = NULL,
},

@jhawthorn jhawthorn Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[Re: line +33]

Do you think you can do this?

#ifdef HAVE_RUBY_TYPED_EMBEDDABLE
        .dfree = RUBY_DEFAULT_FREE,
#else
        .dfree = handle_free,
#end

See this comment inline on Graphite.

@jhawthorn jhawthorn Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That helps us a lot in the GC because it makes the object freeable without calling the handle free function (which is a no-op thanks to embedding)

@amomchilov amomchilov Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think so! And then handle_free doesn't have to have the ifdef i just put in it

.parent = NULL,
.data = NULL,
.flags = RUBY_TYPED_FREE_IMMEDIATELY,
.flags = RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_EMBEDDABLE,
};

static VALUE rdxr_handle_alloc(VALUE klass) {
Expand Down
17 changes: 17 additions & 0 deletions ext/rubydex/ruby_compat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#ifndef RUBYDEX_RUBY_COMPAT_H
#define RUBYDEX_RUBY_COMPAT_H

#include "ruby.h"

#ifdef RUBY_TYPED_EMBEDDABLE
# define HAVE_RUBY_TYPED_EMBEDDABLE 1
#else
# ifdef HAVE_CONST_RUBY_TYPED_EMBEDDABLE
# define RUBY_TYPED_EMBEDDABLE RUBY_TYPED_EMBEDDABLE
# define HAVE_RUBY_TYPED_EMBEDDABLE 1
# else
# define RUBY_TYPED_EMBEDDABLE 0
# endif
#endif

#endif // RUBYDEX_RUBY_COMPAT_H
Loading