From 839bf5fc774b8e9a6e7d42c12a5ada1df61b8df4 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 1 Mar 2023 16:07:33 -0800 Subject: [PATCH] shape iv ancestor search This implements +rb_shape_get_iv_index_with_hint: a version of rb_shape_get_iv_index which uses a previous shape with a valid index for the given IV in order to speed up searches on a IV inline cache miss. This works by walking up both the hint shape's and current shape's tree branches in parallel until they meet at a common ancestor. The thinking behind this is that in many cases we have different, but very similar shapes, but who share the common ancestor defining the IV we're looking for. This should also work well for a frozen version of a shape. This is significantly faster when the divergence between the current and hint shape is shallow, but when it is not (there is no relation between the two shapes) it still resorts to a linear scan. The full scan is slower with this, as more work is being done, but I believe not 2x slower (the LL means we'll be stalling all the time on memory so walking two lists in parallel isn't _so_ bad). Benchmark: https://gist.github.com/jhawthorn/e9db7a1443eaa957cef772dccdc14d9a With a shallow tree divergence before: ruby --disable-gems test_shape_search.rb 1.64s user 0.00s system 99% cpu 1.648 total after: ./miniruby test_shape_search.rb 0.67s user 0.00s system 99% cpu 0.666 total With a deep tree divergence (assumed uncommon) before: ruby --disable-gems test_shape_search_deep.rb 1.64s user 0.00s system 99% cpu 1.643 total after: ./miniruby test_shape_search_deep.rb 1.88s user 0.01s system 99% cpu 1.889 total --- shape.c | 37 +++++++++++++++++++++++++++++++++++++ shape.h | 1 + vm_insnhelper.c | 2 +- 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/shape.c b/shape.c index 89a2c3bd0bef8a..3b0d2734210eb5 100644 --- a/shape.c +++ b/shape.c @@ -433,6 +433,43 @@ rb_shape_transition_shape_capa(rb_shape_t* shape) return rb_shape_transition_shape_capa_create(shape, shape->capacity * 2); } +// Same as rb_shape_get_iv_index, but uses a provided valid shape id and index +// to return a result faster if branches of the shape tree are closely related. +bool +rb_shape_get_iv_index_with_hint(rb_shape_t * shape, ID id, attr_index_t *value, shape_id_t shape_id_hint, attr_index_t index_hint) +{ + if (shape_id_hint == INVALID_SHAPE_ID) + return rb_shape_get_iv_index(shape, id, value); + + rb_shape_t * shape_hint = rb_shape_get_shape_by_id(shape_id_hint); + + while (shape->next_iv_index > index_hint) { + while (shape_hint->next_iv_index > shape->next_iv_index) { + shape_hint = rb_shape_get_parent(shape_hint); + } + + if (shape_hint == shape) { + // We've found a common ancestor so use the index hint + if (index_hint == (attr_index_t)-1) { + return false; + } else { + *value = index_hint; + return true; + } + } + if (shape->edge_name == id) { + // We found the matching id before a common ancestor + // Fall through to rb_shape_get_iv_index to check and return the + // correct value. + break; + } + + shape = rb_shape_get_parent(shape); + } + + return rb_shape_get_iv_index(shape, id, value); +} + bool rb_shape_get_iv_index(rb_shape_t * shape, ID id, attr_index_t *value) { diff --git a/shape.h b/shape.h index 1b2221ad769987..67f8f21d420ded 100644 --- a/shape.h +++ b/shape.h @@ -144,6 +144,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); +bool rb_shape_get_iv_index_with_hint(rb_shape_t * shape, ID id, attr_index_t * value, shape_id_t shape_id_hint, attr_index_t index_hint); bool rb_shape_obj_too_complex(VALUE obj); void rb_shape_set_shape(VALUE obj, rb_shape_t* shape); diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 3955a523d03ba0..ee8662f3ab59f7 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -1294,7 +1294,7 @@ vm_getivar(VALUE obj, ID id, const rb_iseq_t *iseq, IVC ic, const struct rb_call } } else { - if (rb_shape_get_iv_index(shape, id, &index)) { + if (rb_shape_get_iv_index_with_hint(shape, id, &index, cached_id, index)) { // 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);