From eebf52446fa627ad399d2cffeb103d2ff6254eaa Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 6 Mar 2023 18:19:47 +0100 Subject: Implement Write Barrier and dsize for FFI::Pointer As well as FFI::MemoryPointer and FFI::AbstractMemory 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. --- ext/ffi_c/AbstractMemory.c | 14 ++++++++++++-- ext/ffi_c/Buffer.c | 35 +++++++++++++++++++++++++++++------ ext/ffi_c/MemoryPointer.c | 20 +++++++++++++++++--- ext/ffi_c/Pointer.c | 26 ++++++++++++++++++++------ spec/ffi/buffer_spec.rb | 10 ++++++++++ spec/ffi/pointer_spec.rb | 8 ++++++++ spec/ffi/rbx/memory_pointer_spec.rb | 8 ++++++++ 7 files changed, 104 insertions(+), 17 deletions(-) diff --git a/ext/ffi_c/AbstractMemory.c b/ext/ffi_c/AbstractMemory.c index 8a4291a..f7fb295 100644 --- a/ext/ffi_c/AbstractMemory.c +++ b/ext/ffi_c/AbstractMemory.c @@ -55,6 +55,7 @@ # define RB_OBJ_STRING(obj) StringValueCStr(obj) #endif +static size_t memsize(const void *data); static inline char* memory_address(VALUE self); VALUE rbffi_AbstractMemoryClass = Qnil; static VALUE NullPointerErrorClass = Qnil; @@ -65,9 +66,11 @@ const rb_data_type_t rbffi_abstract_memory_data_type = { /* extern */ .function = { .dmark = NULL, .dfree = RUBY_TYPED_DEFAULT_FREE, - .dsize = NULL, + .dsize = memsize, }, - .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 @@ -80,6 +83,13 @@ memory_allocate(VALUE klass) return obj; } + +static size_t +memsize(const void *data) +{ + return sizeof(AbstractMemory); +} + #define VAL(x, swap) (unlikely(((memory->flags & MEM_SWAP) != 0)) ? swap((x)) : (x)) #define NUM_OP(name, type, toNative, fromNative, swap) \ diff --git a/ext/ffi_c/Buffer.c b/ext/ffi_c/Buffer.c index e3ed90b..75f4873 100644 --- a/ext/ffi_c/Buffer.c +++ b/ext/ffi_c/Buffer.c @@ -52,16 +52,20 @@ static VALUE buffer_initialize(int argc, VALUE* argv, VALUE self); static void buffer_release(void *data); static void buffer_mark(void *data); static VALUE buffer_free(VALUE self); +static size_t allocated_buffer_memsize(const void *data); +static size_t buffer_memsize(const void *data); static const rb_data_type_t buffer_data_type = { .wrap_struct_name = "FFI::Buffer", .function = { .dmark = buffer_mark, .dfree = RUBY_TYPED_DEFAULT_FREE, - .dsize = NULL, + .dsize = buffer_memsize, }, .parent = &rbffi_abstract_memory_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 const rb_data_type_t allocated_buffer_data_type = { @@ -69,10 +73,12 @@ static const rb_data_type_t allocated_buffer_data_type = { .function = { .dmark = NULL, .dfree = buffer_release, - .dsize = NULL, + .dsize = allocated_buffer_memsize, }, .parent = &buffer_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 }; @@ -85,7 +91,7 @@ buffer_allocate(VALUE klass) VALUE obj; obj = TypedData_Make_Struct(klass, Buffer, &allocated_buffer_data_type, buffer); - buffer->data.rbParent = Qnil; + RB_OBJ_WRITE(obj, &buffer->data.rbParent, Qnil); buffer->memory.flags = MEM_RD | MEM_WR; return obj; @@ -204,7 +210,7 @@ slice(VALUE self, long offset, long len) result->memory.size = len; result->memory.flags = ptr->memory.flags; result->memory.typeSize = ptr->memory.typeSize; - result->data.rbParent = self; + RB_OBJ_WRITE(obj, &result->data.rbParent, self); return obj; } @@ -334,6 +340,23 @@ buffer_mark(void *data) rb_gc_mark(ptr->data.rbParent); } +static size_t +buffer_memsize(const void *data) +{ + return sizeof(Buffer); +} + +static size_t +allocated_buffer_memsize(const void *data) +{ + const Buffer *ptr = (const Buffer *)data; + size_t memsize = sizeof(Buffer); + if ((ptr->memory.flags & MEM_EMBED) == 0 && ptr->data.storage != NULL) { + memsize += ptr->memory.size; + } + return memsize; +} + void rbffi_Buffer_Init(VALUE moduleFFI) { diff --git a/ext/ffi_c/MemoryPointer.c b/ext/ffi_c/MemoryPointer.c index cfdfa60..0a747fb 100644 --- a/ext/ffi_c/MemoryPointer.c +++ b/ext/ffi_c/MemoryPointer.c @@ -40,6 +40,7 @@ static VALUE memptr_allocate(VALUE klass); static void memptr_release(void *data); +static size_t memptr_memsize(const void *data); static VALUE memptr_malloc(VALUE self, long size, long count, bool clear); static VALUE memptr_free(VALUE self); @@ -58,10 +59,12 @@ static const rb_data_type_t memory_pointer_data_type = { .function = { .dmark = NULL, .dfree = memptr_release, - .dsize = NULL, + .dsize = memptr_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 @@ -69,7 +72,7 @@ memptr_allocate(VALUE klass) { Pointer* p; VALUE obj = TypedData_Make_Struct(klass, Pointer, &memory_pointer_data_type, p); - p->rbParent = Qnil; + RB_OBJ_WRITE(obj, &p->rbParent, Qnil); p->memory.flags = MEM_RD | MEM_WR; return obj; @@ -157,6 +160,17 @@ memptr_release(void *data) xfree(ptr); } +static size_t +memptr_memsize(const void *data) +{ + const Pointer *ptr = (const Pointer *)data; + size_t memsize = sizeof(Pointer); + if (ptr->allocated) { + memsize += ptr->memory.size; + } + return memsize; +} + /* * call-seq: from_string(s) * @param [String] s string diff --git a/ext/ffi_c/Pointer.c b/ext/ffi_c/Pointer.c index 3d26555..cfa841e 100644 --- a/ext/ffi_c/Pointer.c +++ b/ext/ffi_c/Pointer.c @@ -43,16 +43,19 @@ VALUE rbffi_NullPointerSingleton = Qnil; static void ptr_release(void *data); static void ptr_mark(void *data); +static size_t ptr_memsize(const void *data); const rb_data_type_t rbffi_pointer_data_type = { /* extern */ .wrap_struct_name = "FFI::Pointer", .function = { .dmark = ptr_mark, .dfree = ptr_release, - .dsize = NULL, + .dsize = ptr_memsize, }, .parent = &rbffi_abstract_memory_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 }; VALUE @@ -70,7 +73,7 @@ rbffi_Pointer_NewInstance(void* addr) p->memory.size = LONG_MAX; p->memory.flags = (addr == NULL) ? 0 : (MEM_RD | MEM_WR); p->memory.typeSize = 1; - p->rbParent = Qnil; + RB_OBJ_WRITE(obj, &p->rbParent, Qnil); return obj; } @@ -82,7 +85,7 @@ ptr_allocate(VALUE klass) VALUE obj; obj = TypedData_Make_Struct(klass, Pointer, &rbffi_pointer_data_type, p); - p->rbParent = Qnil; + RB_OBJ_WRITE(obj, &p->rbParent, Qnil); p->memory.flags = MEM_RD | MEM_WR; return obj; @@ -134,7 +137,7 @@ ptr_initialize(int argc, VALUE* argv, VALUE self) if (rb_obj_is_kind_of(rbAddress, rbffi_PointerClass)) { Pointer* orig; - p->rbParent = rbAddress; + RB_OBJ_WRITE(self, &p->rbParent, rbAddress); TypedData_Get_Struct(rbAddress, Pointer, &rbffi_pointer_data_type, orig); p->memory = orig->memory; } else { @@ -215,7 +218,7 @@ slice(VALUE self, long offset, long size) p->memory.size = size; p->memory.flags = ptr->flags; p->memory.typeSize = ptr->typeSize; - p->rbParent = self; + RB_OBJ_WRITE(retval, &p->rbParent, self); return retval; } @@ -471,6 +474,17 @@ ptr_mark(void *data) rb_gc_mark(ptr->rbParent); } +static size_t +ptr_memsize(const void *data) +{ + const Pointer *ptr = (const Pointer *)data; + size_t memsize = sizeof(Pointer); + if (ptr->allocated) { + memsize += ptr->memory.size; + } + return memsize; +} + void rbffi_Pointer_Init(VALUE moduleFFI) { diff --git a/spec/ffi/buffer_spec.rb b/spec/ffi/buffer_spec.rb index 619cb6b..e031f7c 100644 --- a/spec/ffi/buffer_spec.rb +++ b/spec/ffi/buffer_spec.rb @@ -285,3 +285,13 @@ describe "Buffer#initialize" do expect(block_executed).to be true end end + +describe "Buffer#memsize_of" do + it "has a memsize function", skip: RUBY_ENGINE != "ruby" do + base_size = ObjectSpace.memsize_of(Object.new) + + buf = FFI::Buffer.new 14 + size = ObjectSpace.memsize_of(buf) + expect(size).to be > base_size + end +end diff --git a/spec/ffi/pointer_spec.rb b/spec/ffi/pointer_spec.rb index 9c85da7..c8242c7 100644 --- a/spec/ffi/pointer_spec.rb +++ b/spec/ffi/pointer_spec.rb @@ -381,5 +381,13 @@ describe "AutoPointer" do expect(mptr[1].read_uint).to eq(0xcafebabe) end end + + it "has a memsize function", skip: RUBY_ENGINE != "ruby" do + base_size = ObjectSpace.memsize_of(Object.new) + + pointer = FFI::Pointer.new(:int, 0xdeadbeef) + size = ObjectSpace.memsize_of(pointer) + expect(size).to be > base_size + end end diff --git a/spec/ffi/rbx/memory_pointer_spec.rb b/spec/ffi/rbx/memory_pointer_spec.rb index 3878973..2db89a9 100644 --- a/spec/ffi/rbx/memory_pointer_spec.rb +++ b/spec/ffi/rbx/memory_pointer_spec.rb @@ -189,4 +189,12 @@ describe "MemoryPointer" do end expect(block_executed).to be true end + + it "has a memsize function", skip: RUBY_ENGINE != "ruby" do + base_size = ObjectSpace.memsize_of(Object.new) + + pointer = FFI::MemoryPointer.from_string("FFI is Awesome") + size = ObjectSpace.memsize_of(pointer) + expect(size).to be > base_size + end end -- cgit v1.2.1