From 2083fa89fc29005035c1a098185c4b707686a437 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 6 Aug 2025 17:39:24 +0200 Subject: [PATCH] Implement `gen_fields_tbl` cache There is a high likelyhood that `rb_obj_fields` is called consecutively for the same object. If we keep a cache of the last IMEMO/fields we interacted with, we can save having to lookup the `gen_fields_tbl`, synchronize the VM lock, etc. On yjit-bench's, I instrumented the hit rate of this cache at: - `shipit`: 38%, with 111k hits. - `lobsters`: 59%, with 367k hits. - `rubocop`: 100% with only 300 hits. I also ran a micro-benchmark which shows that ivar access is: - 1.25x faster when the cache is hit in single ractor mode. - 2x faster when the cache is hit in multi ractor mode. - 1.06x slower when the cache miss in single ractor mode. - 1.01x slower when the cache miss in multi ractor mode. ```yml prelude: | class GenIvar < Array def initialize(...) super @iv = 1 end attr_reader :iv end a = GenIvar.new b = GenIvar.new benchmark: hit: a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; a.iv; miss: a.iv; b.iv; a.iv; b.iv; a.iv; b.iv; a.iv; b.iv; a.iv; b.iv; a.iv; b.iv; a.iv; b.iv; a.iv; b.iv; a.iv; b.iv; a.iv; b.iv; ``` Single ractor: ``` compare-ruby: ruby 3.5.0dev (2025-08-12T02:14:57Z master 428937a536) +YJIT +PRISM [arm64-darwin24] built-ruby: ruby 3.5.0dev (2025-08-12T09:25:35Z gen-fields-cache 9456c35893) +YJIT +PRISM [arm64-darwin24] warming up.. | |compare-ruby|built-ruby| |:-----|-----------:|---------:| |hit | 4.090M| 5.121M| | | -| 1.25x| |miss | 3.756M| 3.534M| | | 1.06x| -| ``` Multi-ractor: ``` compare-ruby: ruby 3.5.0dev (2025-08-12T02:14:57Z master 428937a536) +YJIT +PRISM [arm64-darwin24] built-ruby: ruby 3.5.0dev (2025-08-12T09:25:35Z gen-fields-cache 9456c35893) +YJIT +PRISM [arm64-darwin24] warming up.. | |compare-ruby|built-ruby| |:-----|-----------:|---------:| |hit | 2.205M| 4.460M| | | -| 2.02x| |miss | 2.117M| 2.094M| | | 1.01x| -| ``` --- variable.c | 41 +++++++++++++++++++++++++++++++++-------- vm.c | 6 ++++++ vm_core.h | 5 +++++ 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/variable.c b/variable.c index 0c596d5c9e..4f0f83d203 100644 --- a/variable.c +++ b/variable.c @@ -1245,9 +1245,19 @@ rb_obj_fields(VALUE obj, ID field_name) goto generic_fields; default: generic_fields: - RB_VM_LOCKING() { - if (!st_lookup(generic_fields_tbl_, (st_data_t)obj, (st_data_t *)&fields_obj)) { - rb_bug("Object is missing entry in generic_fields_tbl"); + { + rb_execution_context_t *ec = GET_EC(); + if (ec->gen_fields_cache.obj == obj) { + fields_obj = ec->gen_fields_cache.fields_obj; + } + else { + RB_VM_LOCKING() { + if (!st_lookup(generic_fields_tbl_, (st_data_t)obj, (st_data_t *)&fields_obj)) { + rb_bug("Object is missing entry in generic_fields_tbl"); + } + } + ec->gen_fields_cache.fields_obj = fields_obj; + ec->gen_fields_cache.obj = obj; } } } @@ -1275,8 +1285,15 @@ rb_free_generic_ivar(VALUE obj) goto generic_fields; default: generic_fields: - RB_VM_LOCKING() { - st_delete(generic_fields_tbl_no_ractor_check(), &key, &value); + { + rb_execution_context_t *ec = GET_EC(); + if (ec->gen_fields_cache.obj == obj) { + ec->gen_fields_cache.obj = Qundef; + ec->gen_fields_cache.fields_obj = Qundef; + } + RB_VM_LOCKING() { + st_delete(generic_fields_tbl_no_ractor_check(), &key, &value); + } } } RBASIC_SET_SHAPE_ID(obj, ROOT_SHAPE_ID); @@ -1307,10 +1324,18 @@ rb_obj_set_fields(VALUE obj, VALUE fields_obj, ID field_name, VALUE original_fie goto generic_fields; default: generic_fields: - RB_VM_LOCKING() { - st_insert(generic_fields_tbl_, (st_data_t)obj, (st_data_t)fields_obj); + { + RB_VM_LOCKING() { + st_insert(generic_fields_tbl_, (st_data_t)obj, (st_data_t)fields_obj); + } + RB_OBJ_WRITTEN(obj, original_fields_obj, fields_obj); + + rb_execution_context_t *ec = GET_EC(); + if (ec->gen_fields_cache.fields_obj != fields_obj) { + ec->gen_fields_cache.obj = obj; + ec->gen_fields_cache.fields_obj = fields_obj; + } } - RB_OBJ_WRITTEN(obj, original_fields_obj, fields_obj); } if (original_fields_obj) { diff --git a/vm.c b/vm.c index 4223c2d2ac..479b3be94f 100644 --- a/vm.c +++ b/vm.c @@ -3441,6 +3441,9 @@ rb_execution_context_update(rb_execution_context_t *ec) } ec->storage = rb_gc_location(ec->storage); + + ec->gen_fields_cache.obj = rb_gc_location(ec->gen_fields_cache.obj); + ec->gen_fields_cache.fields_obj = rb_gc_location(ec->gen_fields_cache.fields_obj); } static enum rb_id_table_iterator_result @@ -3505,6 +3508,9 @@ rb_execution_context_mark(const rb_execution_context_t *ec) rb_gc_mark(ec->private_const_reference); rb_gc_mark_movable(ec->storage); + + rb_gc_mark_weak((VALUE *)&ec->gen_fields_cache.obj); + rb_gc_mark_weak((VALUE *)&ec->gen_fields_cache.fields_obj); } void rb_fiber_mark_self(rb_fiber_t *fib); diff --git a/vm_core.h b/vm_core.h index 569aebaba4..b5a04101ab 100644 --- a/vm_core.h +++ b/vm_core.h @@ -1070,6 +1070,11 @@ struct rb_execution_context_struct { VALUE private_const_reference; + struct { + VALUE obj; + VALUE fields_obj; + } gen_fields_cache; + /* for GC */ struct { VALUE *stack_start;