diff options
author | Jean Boussier <byroot@ruby-lang.org> | 2023-11-23 09:44:47 +0100 |
---|---|---|
committer | Jean Boussier <jean.boussier@gmail.com> | 2023-11-23 11:19:12 +0100 |
commit | f1c32c0ee08e924e202f529ee8438d2de4f2702a (patch) | |
tree | 4253b8e4068012185a32d75113a93162b0e5ad81 | |
parent | 784fdecc4c9f6ba9a8fc872518872ed6bdbc6670 (diff) | |
download | ruby-f1c32c0ee08e924e202f529ee8438d2de4f2702a.tar.gz |
vm_setivar_slowpath: only optimize T_OBJECT
We've seen occasional CI failures on i686 in this codepath:
```
[BUG] vm_setivar_slowpath: didn't find ivar @verify_depth in shape
```
Generic ivars are very complex to get right, but also quite rare.
I don't see a good reason to take the risk to give them an optimized
path here, when the much more common T_CLASS/T_MODULE don't have one.
Having an optimization here means duplicating the fairly brittle
logic, which is a recipe for bugs, and I don't think it's worth
it in such case.
-rw-r--r-- | vm_insnhelper.c | 44 |
1 files changed, 8 insertions, 36 deletions
diff --git a/vm_insnhelper.c b/vm_insnhelper.c index d961e51ffd..9f9d0fcfd8 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -1385,47 +1385,19 @@ vm_setivar_slowpath(VALUE obj, ID id, VALUE val, const rb_iseq_t *iseq, IVC ic, #if OPT_IC_FOR_IVAR RB_DEBUG_COUNTER_INC(ivar_set_ic_miss); - switch (BUILTIN_TYPE(obj)) { - case T_OBJECT: - { - rb_check_frozen_internal(obj); + if (BUILTIN_TYPE(obj) == T_OBJECT) { + rb_check_frozen_internal(obj); - attr_index_t index = rb_obj_ivar_set(obj, id, val); + attr_index_t index = rb_obj_ivar_set(obj, id, val); - shape_id_t next_shape_id = ROBJECT_SHAPE_ID(obj); - - if (next_shape_id != OBJ_TOO_COMPLEX_SHAPE_ID) { - populate_cache(index, next_shape_id, id, iseq, ic, cc, is_attr); - } + shape_id_t next_shape_id = ROBJECT_SHAPE_ID(obj); - RB_DEBUG_COUNTER_INC(ivar_set_obj_miss); - return val; + if (next_shape_id != OBJ_TOO_COMPLEX_SHAPE_ID) { + populate_cache(index, next_shape_id, id, iseq, ic, cc, is_attr); } - case T_CLASS: - case T_MODULE: - break; - default: - { - rb_ivar_set(obj, id, val); - shape_id_t next_shape_id = rb_shape_get_shape_id(obj); - if (next_shape_id != OBJ_TOO_COMPLEX_SHAPE_ID) { - rb_shape_t *next_shape = rb_shape_get_shape_by_id(next_shape_id); - attr_index_t index; - - if (rb_shape_get_iv_index(next_shape, id, &index)) { // based off the hash stored in the transition tree - if (index >= MAX_IVARS) { - rb_raise(rb_eArgError, "too many instance variables"); - } - - populate_cache(index, next_shape_id, id, iseq, ic, cc, is_attr); - } - else { - rb_bug("vm_setivar_slowpath: didn't find ivar %s in shape", rb_id2name(id)); - } - } - return val; - } + RB_DEBUG_COUNTER_INC(ivar_set_obj_miss); + return val; } #endif return rb_ivar_set(obj, id, val); |