Optimize allocations with embedded TypedData#852
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
amomchilov
changed the title
ruby_compat.h
amomchilov
force-pushed
the
Alex/embed-graph-alloc
branch
from
19b682a to
64dd2dd
Compare
| 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). |
There was a problem hiding this comment.
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.
| @@ -31,7 +36,7 @@ static const rb_data_type_t handle_type = { | |||
| }, | |||
There was a problem hiding this comment.
[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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
I think so! And then handle_free doesn't have to have the ifdef i just put in it
|
|
||
| #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. |
There was a problem hiding this comment.
nit: just to make it clearer which GC it means (I know Rust doesn't have GC, but maybe now everyone reading this knows)
| // 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. |
| if (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. |
There was a problem hiding this comment.
| // 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. |
amomchilov
force-pushed
the
Alex/Ruby-owned-graph-alloc
branch
2 times, most recently
from
efeb7e0 to
69e5332
Compare
amomchilov
force-pushed
the
Alex/embed-graph-alloc
branch
from
64dd2dd to
35513df
Compare
|
Benchmarked this and got paradoxical results showing that it got slower, not faster. Moving to draft for now. |
Co-authored-by: John Hawthorn <john@hawthorn.email>
amomchilov
force-pushed
the
Alex/embed-graph-alloc
branch
from
d3fc45d to
d14b96b
Compare
amomchilov
force-pushed
the
Alex/Ruby-owned-graph-alloc
branch
from
69e5332 to
c9fd983
Compare
4 participants
Closes #839