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

Optimize super as a bareword #1208

Merged
merged 3 commits into from Nov 29, 2015
Merged

Optimize super as a bareword #1208

merged 3 commits into from Nov 29, 2015

Conversation

jgaskins
Copy link
Contributor

Currently, it's hardcoded in the compiler to use $slice.call(arguments, 0). This PR just does the usual arguments->array copy via iteration while also making the JS code less terrible than I've been doing by declaring that target variable with the rest of the locals. :-)

Results from rake bench (from left to right: master, this PR, MRI):

test/cruby/benchmark/bm_vm2_zsuper.rb              13.864  1.780   0.547

As a bonus, I renamed the index variable for splat args and added a local variable for it. I hadn't intended to commit it but I apparently wasn't paying attention. :-) The only reason I changed it at all was because I originally used arg_index in both places, but Struct.new uses both splat args and bareword super so var arg_index in two places within the same scope caused JSHint to barf.

@elia
Copy link
Member

elia commented Nov 29, 2015

I'd use "$zuper_index" and "$splat_index" to avoid potential conflicts with local vars

@jgaskins
Copy link
Contributor Author

Ah, good idea.

@jgaskins
Copy link
Contributor Author

Alrighty, preceding dollar signs added, ready to go.

meh added a commit that referenced this pull request Nov 29, 2015
@meh meh merged commit 85c74f0 into opal:master Nov 29, 2015
@meh
Copy link
Member

meh commented Nov 29, 2015

👍

hmdne pushed a commit to hmdne/opal that referenced this pull request Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants