Skip to content

Commit 69e5332

Browse files
committed
Allocate Graph memory from Ruby GC
Previously, the Graph objects were initialized in a new Rust-allocated Box. Let's switch to allocating this memory from the Ruby GC. This has several benefits: 1. Like `xmalloc()`, it gives the GC a chance to run if there isn't much free memory available 2. It puts us on the path to using `RUBY_TYPED_EMBEDDABLE` for more optimal allocations
1 parent 393d740 commit 69e5332

3 files changed

Lines changed: 40 additions & 20 deletions

File tree

ext/rubydex/graph.c

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,14 @@ static const char *extract_self_receiver(VALUE opts) {
3838
return StringValueCStr(kwarg_val);
3939
}
4040

41-
// Free function for the custom Graph allocator. We always have to call into Rust to free data allocated by it
41+
// Free function for the custom Graph allocator.
4242
static void graph_free(void *ptr) {
4343
if (ptr) {
44-
rdx_graph_free(ptr);
44+
// let the Rust side drop the Graph struct internally.
45+
rdx_graph_drop(ptr);
46+
47+
// Free the TypeData Ruby object itself
48+
xfree(ptr);
4549
}
4650
}
4751

@@ -61,8 +65,13 @@ const rb_data_type_t graph_type = {
6165
// Custom allocator for the Graph class. Calls into Rust to create a new `Arc<Mutex<Graph>>` that gets stored internally
6266
// as a void pointer
6367
static VALUE rdxr_graph_alloc(VALUE klass) {
64-
void *graph = rdx_graph_new();
65-
return TypedData_Wrap_Struct(klass, &graph_type, graph);
68+
void *graph;
69+
// Can't use `TypedData_Make_Struct`, because the Graph is a Rust type that isn't exposed directly to C.
70+
VALUE graph_obj = rb_data_typed_object_make(klass, &graph_type, &graph, RDX_GRAPH_SIZE);
71+
72+
rdx_graph_init(graph);
73+
74+
return graph_obj;
6675
}
6776

6877
// Graph#index_all: (Array[String] file_paths) -> Array[String]

ext/rubydex/handle.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,15 @@ static const rb_data_type_t handle_type = {
3535
};
3636

3737
static VALUE rdxr_handle_alloc(VALUE klass) {
38-
HandleData *data = ALLOC(HandleData);
38+
HandleData *data;
39+
VALUE handle_obj = TypedData_Make_Struct(klass, HandleData, &handle_type, data);
3940

4041
*data = (HandleData) {
4142
.graph_obj = Qnil,
4243
.id = 0,
4344
};
4445

45-
return TypedData_Wrap_Struct(klass, &handle_type, data);
46+
return handle_obj;
4647
}
4748

4849
static VALUE rdxr_handle_initialize(VALUE self, VALUE graph_obj, VALUE id_val) {

rust/rubydex-sys/src/graph_api.rs

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,38 +23,48 @@ use std::{mem, ptr};
2323

2424
pub type GraphPointer = *mut c_void;
2525

26-
/// Creates a new graph within a mutex. This is meant to be used when creating new Graph objects in Ruby
26+
/// Returns the number of bytes needed to store a Graph in externally allocated memory.
2727
#[unsafe(no_mangle)]
28-
pub extern "C" fn rdx_graph_new() -> GraphPointer {
29-
Box::into_raw(Box::new(Graph::new())) as GraphPointer
28+
pub static RDX_GRAPH_SIZE: usize = mem::size_of::<Graph>();
29+
30+
/// Initializes a Graph in-place at the given `pointer`.
31+
///
32+
/// # Safety
33+
///
34+
/// `pointer` must point to valid, properly aligned memory of at least `RDX_GRAPH_SIZE` bytes.
35+
#[unsafe(no_mangle)]
36+
pub unsafe extern "C" fn rdx_graph_init(pointer: GraphPointer) {
37+
unsafe {
38+
pointer.cast::<Graph>().write(Graph::new());
39+
}
3040
}
3141

32-
/// Frees a Graph through its pointer
42+
/// Drops a Graph initialized by `rdx_graph_init` without freeing the underlying memory.
43+
///
44+
/// # Safety
45+
///
46+
/// `pointer` must point to a valid initialized Graph.
3347
#[unsafe(no_mangle)]
34-
pub extern "C" fn rdx_graph_free(pointer: GraphPointer) {
48+
pub unsafe extern "C" fn rdx_graph_drop(pointer: GraphPointer) {
3549
unsafe {
36-
let _ = Box::from_raw(pointer.cast::<Graph>());
50+
ptr::drop_in_place(pointer.cast::<Graph>());
3751
}
3852
}
3953

4054
pub fn with_graph<F, T>(pointer: GraphPointer, action: F) -> T
4155
where
4256
F: FnOnce(&Graph) -> T,
4357
{
44-
let mut graph = unsafe { Box::from_raw(pointer.cast::<Graph>()) };
45-
let result = action(&mut graph);
46-
mem::forget(graph);
47-
result
58+
let graph = unsafe { &*pointer.cast::<Graph>() };
59+
action(graph)
4860
}
4961

5062
fn with_mut_graph<F, T>(pointer: GraphPointer, action: F) -> T
5163
where
5264
F: FnOnce(&mut Graph) -> T,
5365
{
54-
let mut graph = unsafe { Box::from_raw(pointer.cast::<Graph>()) };
55-
let result = action(&mut graph);
56-
mem::forget(graph);
57-
result
66+
let graph = unsafe { &mut *pointer.cast::<Graph>() };
67+
action(graph)
5868
}
5969

6070
/// Searches the graph using exact substring matching

0 commit comments

Comments
 (0)