From c05fa0293f2e624cd9e78817e9d6bc4d2b039a8a Mon Sep 17 00:00:00 2001
From: Jean Boussier <jean.boussier@gmail.com>
Date: Mon, 6 Mar 2023 12:17:13 +0100
Subject: Implement Write Barrier and dsize for FFI::DynamicLibrary

And FFI::DynamicLibrary::Symbol

Ref: https://github.com/ffi/ffi/pull/991

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

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

This commit also implement a `dsize` function so that these instance report
a more relevant size in various memory profilers. It's not counting everything
because some types are opaque right now, so a larger refactoring would be needed.

While I was at it, I moved `Symbol.library` into the unused `Symbol.base.rbParent`
which seemed appropriate and saves a pointer.
---
 ext/ffi_c/DynamicLibrary.c | 26 ++++++++++++++++----------
 spec/ffi/library_spec.rb   | 17 +++++++++++++++++
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/ext/ffi_c/DynamicLibrary.c b/ext/ffi_c/DynamicLibrary.c
index 108394c..6e64c57 100644
--- a/ext/ffi_c/DynamicLibrary.c
+++ b/ext/ffi_c/DynamicLibrary.c
@@ -50,7 +50,6 @@
 
 typedef struct LibrarySymbol_ {
     Pointer base;
-    VALUE library;
     VALUE name;
 } LibrarySymbol;
 
@@ -62,6 +61,7 @@ static void library_free(void *);
 static VALUE symbol_allocate(VALUE klass);
 static VALUE symbol_new(VALUE library, void* address, VALUE name);
 static void symbol_mark(void *data);
+static size_t symbol_memsize(const void *data);
 
 static const rb_data_type_t rbffi_library_data_type = {
     .wrap_struct_name = "FFI::DynamicLibrary",
@@ -78,10 +78,12 @@ static const rb_data_type_t library_symbol_data_type = {
     .function = {
         .dmark = symbol_mark,
         .dfree = RUBY_TYPED_DEFAULT_FREE,
-        .dsize = NULL,
+        .dsize = symbol_memsize,
     },
     .parent = &rbffi_pointer_data_type,
-    .flags = RUBY_TYPED_FREE_IMMEDIATELY
+    // IMPORTANT: WB_PROTECTED objects must only use the RB_OBJ_WRITE()
+    // macro to update VALUE references, as to trigger write barriers.
+    .flags = RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED
 };
 
 static VALUE LibraryClass = Qnil, SymbolClass = Qnil;
@@ -223,9 +225,8 @@ symbol_allocate(VALUE klass)
 {
     LibrarySymbol* sym;
     VALUE obj = TypedData_Make_Struct(klass, LibrarySymbol, &library_symbol_data_type, sym);
-    sym->name = Qnil;
-    sym->library = Qnil;
-    sym->base.rbParent = Qnil;
+    RB_OBJ_WRITE(obj, &sym->base.rbParent, Qnil);
+    RB_OBJ_WRITE(obj, &sym->name, Qnil);
 
     return obj;
 }
@@ -254,8 +255,8 @@ symbol_new(VALUE library, void* address, VALUE name)
     sym->base.memory.size = LONG_MAX;
     sym->base.memory.typeSize = 1;
     sym->base.memory.flags = MEM_RD | MEM_WR;
-    sym->library = library;
-    sym->name = name;
+    RB_OBJ_WRITE(obj, &sym->base.rbParent, library);
+    RB_OBJ_WRITE(obj, &sym->name, name);
 
     return obj;
 }
@@ -264,10 +265,16 @@ static void
 symbol_mark(void *data)
 {
     LibrarySymbol *sym = (LibrarySymbol *)data;
-    rb_gc_mark(sym->library);
+    rb_gc_mark(sym->base.rbParent);
     rb_gc_mark(sym->name);
 }
 
+static size_t
+symbol_memsize(const void *data)
+{
+    return sizeof(LibrarySymbol);
+}
+
 /*
  * call-seq: inspect
  * @return [String]
@@ -356,4 +363,3 @@ rbffi_DynamicLibrary_Init(VALUE moduleFFI)
 #undef DEF
 
 }
-
diff --git a/spec/ffi/library_spec.rb b/spec/ffi/library_spec.rb
index 5b806bb..a17d673 100644
--- a/spec/ffi/library_spec.rb
+++ b/spec/ffi/library_spec.rb
@@ -322,4 +322,21 @@ describe "Library" do
       expect(val[:data]).to eq(i)
     end
   end
+
+  describe "Symbol" do
+    before do
+      @libtest = FFI::DynamicLibrary.open(
+        TestLibrary::PATH,
+        FFI::DynamicLibrary::RTLD_LAZY | FFI::DynamicLibrary::RTLD_GLOBAL,
+      )
+    end
+
+    it "has a memsize function", skip: RUBY_ENGINE != "ruby" do
+      base_size = ObjectSpace.memsize_of(Object.new)
+
+      symbol = @libtest.find_symbol("gvar_gstruct_set")
+      size = ObjectSpace.memsize_of(symbol)
+      expect(size).to be > base_size
+    end
+  end
 end
-- 
cgit v1.2.1