Skip to content

Commit

Permalink
YJIT: Support empty splat and some block_arg calls to ivar getters
Browse files Browse the repository at this point in the history
These seem odd at first glance, but they're used with `...` calls with
`Module#delegate` from Active Support. These account for ~3% of fallback
reasons in the `lobsters` benchmark.
  • Loading branch information
XrXr committed Jan 16, 2024
1 parent 88bb09b commit bfb8cc8
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 17 deletions.
34 changes: 33 additions & 1 deletion test/ruby/test_yjit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1514,6 +1514,24 @@ def test_disable_stats
assert_in_out_err(%w[--yjit-stats --yjit-disable])
end

def test_odd_calls_to_attr_reader
# Use of delegate from ActiveSupport use these kind of calls to getter methods.
assert_compiles(<<~RUBY, result: [1, 1, 1], no_send_fallbacks: true)
class One
attr_reader :one
def initialize
@one = 1
end
end
def calls(obj, empty, &)
[obj.one(*empty), obj.one(&), obj.one(*empty, &)]
end
calls(One.new, [])
RUBY
end

private

def code_gc_helpers
Expand All @@ -1537,7 +1555,17 @@ def assert_no_exits(script)
end

ANY = Object.new
def assert_compiles(test_script, insns: [], call_threshold: 1, stdout: nil, exits: {}, result: ANY, frozen_string_literal: nil, mem_size: nil, code_gc: false)
def assert_compiles(
test_script, insns: [],
call_threshold: 1,
stdout: nil,
exits: {},
result: ANY,
frozen_string_literal: nil,
mem_size: nil,
code_gc: false,
no_send_fallbacks: false
)
reset_stats = <<~RUBY
RubyVM::YJIT.runtime_stats
RubyVM::YJIT.reset_stats!
Expand Down Expand Up @@ -1610,6 +1638,10 @@ def collect_insns(iseq)
end
end

if no_send_fallbacks
assert_equal(0, runtime_stats[:num_send_dynamic], "Expected no use of fallback implementation")
end

# Only available when --enable-yjit=dev
if runtime_stats[:all_stats]
missed_insns = insns.dup
Expand Down
50 changes: 34 additions & 16 deletions yjit/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7307,21 +7307,43 @@ fn gen_send_general(
);
}
VM_METHOD_TYPE_IVAR => {
if flags & VM_CALL_ARGS_SPLAT != 0 {
gen_counter_incr(asm, Counter::send_args_splat_ivar);
// This is a .send call not supported right now for getters
if flags & VM_CALL_OPT_SEND != 0 {
gen_counter_incr(asm, Counter::send_send_getter);
return None;
}

if argc != 0 {
// Argument count mismatch. Getters take no arguments.
gen_counter_incr(asm, Counter::send_getter_arity);
return None;
if flags & VM_CALL_ARGS_BLOCKARG != 0 {
match asm.ctx.get_opnd_type(StackOpnd(0)) {
Type::Nil | Type::BlockParamProxy => {
// Getters ignore the block arg, and these types of block args can be
// passed without side-effect (never any `to_proc` call).
asm.stack_pop(1);
}
_ => {
gen_counter_incr(asm, Counter::send_getter_block_arg);
return None;
}
}
}

// This is a .send call not supported right now for getters
if flags & VM_CALL_OPT_SEND != 0 {
gen_counter_incr(asm, Counter::send_send_getter);
return None;
if argc != 0 {
// Guard for simple splat of empty array
if VM_CALL_ARGS_SPLAT == flags & (VM_CALL_ARGS_SPLAT | VM_CALL_KWARG | VM_CALL_KW_SPLAT)
&& argc == 1 {
// Not using chain guards since on failure these likely end up just raising
// ArgumentError
let splat = asm.stack_opnd(0);
guard_object_is_array(asm, splat, splat.into(), Counter::guard_send_getter_splat_non_empty);
let splat_len = get_array_len(asm, splat);
asm.cmp(splat_len, 0.into());
asm.jne(Target::side_exit(Counter::guard_send_getter_splat_non_empty));
asm.stack_pop(1);
} else {
// Argument count mismatch. Getters take no arguments.
gen_counter_incr(asm, Counter::send_getter_arity);
return None;
}
}

if c_method_tracing_currently_enabled(jit) {
Expand All @@ -7338,13 +7360,9 @@ fn gen_send_general(
return None;
}

let recv = asm.stack_opnd(0); // the receiver should now be the stack top
let ivar_name = unsafe { get_cme_def_body_attr_id(cme) };

if flags & VM_CALL_ARGS_BLOCKARG != 0 {
gen_counter_incr(asm, Counter::send_getter_block_arg);
return None;
}

return gen_get_ivar(
jit,
asm,
Expand All @@ -7353,7 +7371,7 @@ fn gen_send_general(
comptime_recv,
ivar_name,
recv,
recv_opnd,
recv.into(),
);
}
VM_METHOD_TYPE_ATTRSET => {
Expand Down
1 change: 1 addition & 0 deletions yjit/src/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ make_counters! {

// Method calls that exit to the interpreter
guard_send_block_arg_type,
guard_send_getter_splat_non_empty,
guard_send_klass_megamorphic,
guard_send_se_cf_overflow,
guard_send_se_protected_check_failed,
Expand Down

0 comments on commit bfb8cc8

Please sign in to comment.