Skip to content
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

YJIT: expandarray for non-arrays #9495

Merged
merged 3 commits into from
Jan 12, 2024
Merged

Conversation

ywenc
Copy link
Contributor

@ywenc ywenc commented Jan 11, 2024

This PR updates the expandarray instruction for YJIT.

expandarray is an instruction that appears for array decomposition or multiple assignment.

Most commonly, expandarray will be encountered when assigning multiple variables at once from array,
example -

a, b, c = [1, 2, 3]

But it can often appear with non-arrays, when assigning multiple variables to a single value. It is common enough to see expandarray on a single nil that gen_expandarray previously had a special case if the operand was nil, but this is potentially incorrect because to_ary can be redefined on NilClass.

This PR updates the non-array case by bailing if to_ary is defined on a non-array operand. Otherwise, we push nils to fill the unset values, then push the original operand, which also helps get rid of side exits from a pattern like:

def some_method
   if ok
      true
   else 
      [false, err]
   end
end

value, err = some_method

This pattern was one of the most commonly seen side-exits for GitHub benchmarks.

Quick benchmark:

% time ./miniruby --disable-gems --yjit -e '10_000_000.times { a,b = 1 }'
./miniruby --disable-gems --yjit -e '10_000_000.times { a,b = 1 }'  0.35s user 0.02s system 61% cpu 0.594 total

% time ruby --disable-gems --yjit -e '10_000_000.times { a,b = 1 }'      
ruby --disable-gems --yjit -e '10_000_000.times { a,b = 1 }'  1.01s user 0.03s system 95% cpu 1.098 total

@matzbot matzbot requested a review from a team January 11, 2024 20:49
@maximecb maximecb enabled auto-merge (squash) January 11, 2024 20:58
Co-authored-by: John Hawthorn <john@hawthorn.email>
auto-merge was automatically disabled January 11, 2024 20:59

Head branch was pushed to by a user without write access

bootstraptest/test_yjit.rb Outdated Show resolved Hide resolved
@maximecb maximecb enabled auto-merge (squash) January 12, 2024 15:25
@maximecb maximecb merged commit 16624ef into ruby:master Jan 12, 2024
96 checks passed
@ywenc ywenc deleted the update-expandarray branch January 18, 2024 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants