WIP: vm_getivar handle semi-stale

$ make benchmark ITEM=vm_ivar_memoize COMPARE_RUBY=$(which ruby)
revision.h updated
/usr/bin/ruby --disable=gems -rrubygems -I./benchmark/lib ./benchmark/benchmark-driver/exe/benchmark-driver \
	            --executables="compare-ruby::/opt/rubies/head/bin/ruby -I.ext/common --disable-gem" \
	            --executables="built-ruby::./miniruby -I./lib -I. -I.ext/common  ./tool/runruby.rb --extout=.ext  -- --disable-gems --disable-gem" \
	            --output=markdown --output-compare -v $(find ./benchmark -maxdepth 1 -name 'vm_ivar_memoize' -o -name '*vm_ivar_memoize*.yml' -o -name '*vm_ivar_memoize*.rb' | sort)
compare-ruby: ruby 3.3.0dev (2023-10-10T02:05:29Z shape-cache-miss-i.. 5245123) [arm64-darwin22]
built-ruby: ruby 3.3.0dev (2023-10-10T09:03:22Z shape-cache-miss-i.. 6c426d75d8) [arm64-darwin22]

|                                |compare-ruby|built-ruby|
|:-------------------------------|-----------:|---------:|
|vm_ivar_stable_shape            |     12.412M|   12.409M|
|                                |       1.00x|         -|
|vm_ivar_memoize_unstable_shape  |      1.236M|    1.772M|
|                                |           -|     1.43x|

Author: byroot

benchmark/vm_ivar_memoize.yml

@@ -0,0 +1,46 @@
+prelude: |
+  class Record
+    def initialize
+      @first = 0
+      68.times do |i|
+        instance_variable_set("@ivar_#{i}", i)
+      end
+    end
+
+    def first
+      @first
+    end
+
+    def lazy_set
+      @uninitialized ||= 123
+    end
+
+    def lazy_set2
+      @uninitialized2 ||= 123
+    end
+  end
+
+  Record.new # Need one alloc to right size
+
+  BASE = Record.new
+  LAZY1 = Record.new
+  LAZY1.lazy_set
+  LAZY2 = Record.new
+  LAZY2.lazy_set2
+
+benchmark:
+  vm_ivar_stable_shape: |
+    BASE.first
+    BASE.first
+    BASE.first
+    BASE.first
+    BASE.first
+    BASE.first
+  vm_ivar_memoize_unstable_shape: |
+    BASE.first
+    LAZY1.first
+    LAZY2.first
+    BASE.first
+    LAZY1.first
+    LAZY2.first
+loop_count: 5_000_000

shape.c

@@ -427,6 +427,28 @@ rb_shape_transition_shape_capa(rb_shape_t* shape, uint32_t new_capacity)
     return new_shape;
 }
 
+static inline bool
+shape_get_iv_index(rb_shape_t * shape, attr_index_t *value)
+{
+    enum shape_type shape_type;
+    shape_type = (enum shape_type)shape->type;
+
+    switch (shape_type) {
+      case SHAPE_IVAR:
+        RUBY_ASSERT(shape->next_iv_index > 0);
+        *value = shape->next_iv_index - 1;
+        return true;
+      case SHAPE_CAPACITY_CHANGE:
+      case SHAPE_ROOT:
+      case SHAPE_INITIAL_CAPACITY:
+      case SHAPE_T_OBJECT:
+        return false;
+      case SHAPE_OBJ_TOO_COMPLEX:
+      case SHAPE_FROZEN:
+        rb_bug("Ivar should not exist on transition");
+    }
+}
+
 bool
 rb_shape_get_iv_index(rb_shape_t * shape, ID id, attr_index_t *value)
 {
@@ -436,29 +458,43 @@ rb_shape_get_iv_index(rb_shape_t * shape, ID id, attr_index_t *value)
 
     while (shape->parent_id != INVALID_SHAPE_ID) {
         if (shape->edge_name == id) {
-            enum shape_type shape_type;
-            shape_type = (enum shape_type)shape->type;
-
-            switch (shape_type) {
-              case SHAPE_IVAR:
-                RUBY_ASSERT(shape->next_iv_index > 0);
-                *value = shape->next_iv_index - 1;
-                return true;
-              case SHAPE_CAPACITY_CHANGE:
-              case SHAPE_ROOT:
-              case SHAPE_INITIAL_CAPACITY:
-              case SHAPE_T_OBJECT:
-                return false;
-              case SHAPE_OBJ_TOO_COMPLEX:
-              case SHAPE_FROZEN:
-                rb_bug("Ivar should not exist on transition");
-            }
+            return shape_get_iv_index(shape, value);
         }
         shape = rb_shape_get_parent(shape);
     }
     return false;
 }
 
+long
+rb_shape_get_iv_index_stale(shape_id_t new_shape_id, ID id, attr_index_t *value, shape_id_t old_shape_id)
+{
+    // It doesn't make sense to ask for the index of an IV that's stored
+    // on an object that is "too complex" as it uses a hash for storing IVs
+    RUBY_ASSERT(new_shape_id != OBJ_TOO_COMPLEX_SHAPE_ID);
+    RUBY_ASSERT(old_shape_id != OBJ_TOO_COMPLEX_SHAPE_ID);
+
+    rb_shape_t *shape = rb_shape_get_shape_by_id(new_shape_id);
+
+    // if (old_shape_id == INVALID_SHAPE_ID) {
+    //     return rb_shape_get_iv_index(shape, id, value) ? -1 : 0;
+    // }
+
+    long depth = 1;
+    while (shape->parent_id != INVALID_SHAPE_ID) {
+        if (shape->parent_id == old_shape_id) {
+            shape_get_iv_index(shape, value);
+            return depth;
+        }
+        else if (shape->edge_name == id) {
+            return shape_get_iv_index(shape, value) ? -1 : 0;
+        }
+        depth++;
+        shape = rb_shape_get_parent(shape);
+    }
+
+    return 0;
+}
+
 void
 rb_shape_set_shape(VALUE obj, rb_shape_t* shape)
 {

shape.h

@@ -40,6 +40,8 @@ typedef uint16_t shape_id_t;
 # define SPECIAL_CONST_SHAPE_ID (SIZE_POOL_COUNT * 2)
 # define OBJ_TOO_COMPLEX_SHAPE_ID (SPECIAL_CONST_SHAPE_ID + 1)
 
+# define SHAPE_CLOSE_DESCENDANT_MAX_DEPTH 4
+
 struct rb_shape {
     struct rb_id_table * edges; // id_table from ID (ivar) to next shape
     ID edge_name; // ID (ivar) for transition from parent to rb_shape
@@ -145,6 +147,7 @@ rb_shape_t* rb_shape_get_shape_by_id(shape_id_t shape_id);
 shape_id_t rb_shape_get_shape_id(VALUE obj);
 rb_shape_t * rb_shape_get_next_iv_shape(rb_shape_t * shape, ID id);
 bool rb_shape_get_iv_index(rb_shape_t * shape, ID id, attr_index_t * value);
+long rb_shape_get_iv_index_stale(shape_id_t new_shape_id, ID id, attr_index_t *value, shape_id_t old_shape_id);
 bool rb_shape_obj_too_complex(VALUE obj);
 
 void rb_shape_set_shape(VALUE obj, rb_shape_t* shape);

vm_insnhelper.c

@@ -1286,14 +1286,14 @@ vm_getivar(VALUE obj, ID id, const rb_iseq_t *iseq, IVC ic, const struct rb_call
         }
 #endif
 
-        rb_shape_t *shape = rb_shape_get_shape_by_id(shape_id);
-
         if (shape_id == OBJ_TOO_COMPLEX_SHAPE_ID) {
             if (!st_lookup(ROBJECT_IV_HASH(obj), id, &val)) {
                 val = default_value;
             }
         }
-        else {
+        else if (UNLIKELY(index == ATTR_INDEX_NOT_SET)) {
+            rb_shape_t *shape = rb_shape_get_shape_by_id(shape_id);
+
             if (rb_shape_get_iv_index(shape, id, &index)) {
                 // This fills in the cache with the shared cache object.
                 // "ent" is the shared cache object
@@ -1314,6 +1314,42 @@ vm_getivar(VALUE obj, ID id, const rb_iseq_t *iseq, IVC ic, const struct rb_call
                 val = default_value;
             }
         }
+        else {
+            long ancestor_depth = rb_shape_get_iv_index_stale(shape_id, id, &index, cached_id);
+            switch (ancestor_depth) {
+              case 0: // Not found
+                if (is_attr) {
+                    vm_cc_attr_index_initialize(cc, shape_id);
+                }
+                else {
+                    vm_ic_attr_index_initialize(ic, shape_id);
+                }
+
+                val = default_value;
+                break;
+              case -1: // Found but wasn't an ancestor
+                // This fills in the cache with the shared cache object.
+                // "ent" is the shared cache object
+                fill_ivar_cache(iseq, ic, cc, is_attr, index, shape_id);
+
+                // We fetched the ivar list above
+                val = ivar_list[index];
+                RUBY_ASSERT(!UNDEF_P(val));
+                break;
+              default: // cached_id is an ancestor of shape_id so index is unchanged
+                if (ancestor_depth < SHAPE_CLOSE_DESCENDANT_MAX_DEPTH) {
+                    // if shape_id is a close decendent of cached_id, we assume this
+                    // object might be using the "memoization" pattern, so it will flip-flop
+                    // a lot and we might as well keep the close ancestor in cache.
+                    // TODO: is there a better heuristic? e.g. number of edges?
+                    fill_ivar_cache(iseq, ic, cc, is_attr, index, shape_id);
+                }
+
+                val = ivar_list[index];
+                RUBY_ASSERT(!UNDEF_P(val));
+                break;
+            }
+        }
 
     }