8000 Add SHAPE_ID_HAS_IVAR_MASK for quick ivar check by casperisfine · Pull Request #13606 · ruby/ruby · GitHub
[go: up one dir, main page]

Skip to content

Add SHAPE_ID_HAS_IVAR_MASK for quick ivar check #13606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions internal/string.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ enum ruby_rstring_private_flags {
#endif

/* string.c */
VALUE rb_str_dup_m(VALUE str);
VALUE rb_fstring(VALUE);
VALUE rb_fstring_cstr(const char *str);
VALUE rb_fstring_enc_new(const char *ptr, long len, rb_encoding *enc);
Expand Down
20 changes: 19 additions & 1 deletion shape.c
Original file line number Diff line number Diff line change
Expand Up @@ -1234,6 +1234,23 @@ rb_shape_verify_consistency(VALUE obj, shape_id_t shape_id)
}
}

// Make sure SHAPE_ID_HAS_IVAR_MASK is valid.
if (rb_shape_too_complex_p(shape_id)) {
RUBY_ASSERT(shape_id & SHAPE_ID_HAS_IVAR_MASK);
}
else {
attr_index_t ivar_count = RSHAPE_LEN(shape_id);
if (has_object_id) {
ivar_count--;
}
if (ivar_count) {
RUBY_ASSERT(shape_id & SHAPE_ID_HAS_IVAR_MASK);
}
else {
RUBY_ASSERT(!(shape_id & SHAPE_ID_HAS_IVAR_MASK));
}
}

uint8_t flags_heap_index = rb_shape_heap_index(shape_id);
if (RB_TYPE_P(obj, T_OBJECT)) {
size_t shape_id_slot_size = rb_shape_tree.capacities[flags_heap_index - 1] * sizeof(VALUE) + sizeof(struct RBasic);
Expand Down Expand Up @@ -1524,14 +1541,15 @@ Init_default_shapes(void)
root->type = SHAPE_ROOT;
rb_shape_tree.root_shape = root;
RUBY_ASSERT(raw_shape_id(rb_shape_tree.root_shape) == ROOT_SHAPE_ID);
RUBY_ASSERT(!(raw_shape_id(rb_shape_tree.root_shape) & SHAPE_ID_HAS_IVAR_MASK));

bool dontcare;
rb_shape_t *root_with_obj_id = get_next_shape_internal(root, id_object_id, SHAPE_OBJ_ID, &dontcare, true);
RUBY_ASSERT(raw_shape_id(root_with_obj_id) == ROOT_SHAPE_WITH_OBJ_ID);
RUBY_ASSERT(root_with_obj_id->type == SHAPE_OBJ_ID);
RUBY_ASSERT(root_with_obj_id->edge_name == id_object_id);
RUBY_ASSERT(root_with_obj_id->next_field_index == 1);
(void)root_with_obj_id;
RUBY_ASSERT(!(raw_shape_id(root_with_obj_id) & SHAPE_ID_HAS_IVAR_MASK));
}

void
Expand Down
16 changes: 16 additions & 0 deletions shape.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ STATIC_ASSERT(shape_id_num_bits, SHAPE_ID_NUM_BITS == sizeof(shape_id_t) * CHAR_
#define SHAPE_ID_HEAP_INDEX_MAX ((1 << SHAPE_ID_HEAP_INDEX_BITS) - 1)
#define SHAPE_ID_HEAP_INDEX_MASK (SHAPE_ID_HEAP_INDEX_MAX << SHAPE_ID_HEAP_INDEX_OFFSET)

// This masks allows to check if a shape_id contains any ivar.
// It rely on ROOT_SHAPE_WITH_OBJ_ID==1.
#define SHAPE_ID_HAS_IVAR_MASK (SHAPE_ID_FL_TOO_COMPLEX | (SHAPE_ID_OFFSET_MASK - 1))

// The interpreter doesn't care about frozen status or slot size when reading ivars.
// So we normalize shape_id by clearing these bits to improve cache hits.
// JITs however might care about it.
Expand Down Expand Up @@ -327,6 +331,18 @@ rb_shape_obj_has_id(VALUE obj)
return rb_shape_has_object_id(RBASIC_SHAPE_ID(obj));
}

static inline bool
rb_shape_has_ivars(shape_id_t shape_id)
{
return shape_id & SHAPE_ID_HAS_IVAR_MASK;
}

static inline bool
rb_shape_obj_has_ivars(VALUE obj)
{
return rb_shape_has_ivars(RBASIC_SHAPE_ID(obj));
}

// For ext/objspace
RUBY_SYMBOL_EXPORT_BEGIN
typedef void each_shape_callback(shape_id_t shape_id, void *data);
Expand Down
9 changes: 2 additions & 7 deletions string.c
Original file line number Diff line number Diff line change
Expand Up @@ -388,12 +388,7 @@ fstring_hash(VALUE str)
static inline bool
BARE_STRING_P(VALUE str)
{
if (RBASIC_CLASS(str) != rb_cString) return false;

if (FL_TEST_RAW(str, FL_EXIVAR)) {
return rb_ivar_count(str) == 0;
}
return true;
return RBASIC_CLASS(str) == rb_cString && !rb_shape_obj_has_ivars(str);
}

static inline st_index_t
Expand Down Expand Up @@ -2316,7 +2311,7 @@ VALUE
rb_str_dup_m(VALUE str)
{
if (LIKELY(BARE_STRING_P(str))) {
return str_duplicate(rb_obj_class(str), str);
return str_duplicate(rb_cString, str);
}
else {
return rb_obj_dup(str);
Expand Down
1 change: 1 addition & 0 deletions yjit/bindgen/src/main.rs
E5C3
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ fn main() {
.allowlist_function("rb_obj_as_string_result")
.allowlist_function("rb_str_byte_substr")
.allowlist_function("rb_str_substr_two_fixnums")
.allowlist_function("rb_str_dup_m")

// From include/ruby/internal/intern/parse.h
.allowlist_function("rb_backref_get")
Expand Down
6 changes: 1 addition & 5 deletions yjit/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6275,16 +6275,12 @@ fn jit_rb_str_dup(

jit_prepare_call_with_gc(jit, asm);

// Check !FL_ANY_RAW(str, FL_EXIVAR), which is part of BARE_STRING_P.
let recv_opnd = asm.stack_pop(1);
let recv_opnd = asm.load(recv_opnd);
let flags_opnd = Opnd::mem(64, recv_opnd, RUBY_OFFSET_RBASIC_FLAGS);
asm.test(flags_opnd, Opnd::Imm(RUBY_FL_EXIVAR as i64));
asm.jnz(Target::side_exit(Counter::send_str_dup_exivar));

// Call rb_str_dup
let stack_ret = asm.stack_push(Type::CString);
let ret_opnd = asm.ccall(rb_str_dup as *const u8, vec![recv_opnd]);
let ret_opnd = asm.ccall(rb_str_dup_m as *const u8, vec![recv_opnd]);
asm.mov(stack_ret, ret_opnd);

true
Expand Down
1 change: 1 addition & 0 deletions yjit/src/cruby_bindings.inc.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
0