Skip to content

Commit

Permalink
shape iv ancestor search
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jhawthorn authored and byroot committed Oct 16, 2023
1 parent 8f33d80 commit 839bf5f
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 1 deletion.
37 changes: 37 additions & 0 deletions shape.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
1 change: 1 addition & 0 deletions shape.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion vm_insnhelper.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 839bf5f

Please sign in to comment.