-
Notifications
You must be signed in to change notification settings - Fork 194
Fix last super spec #2104
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
Fix last super spec #2104
Conversation
|
||
final Object[] restArgs; | ||
|
||
if (restArg instanceof RubyArray) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we profile this condition (with ConditionProfile)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could do - but I'll be doing a check of the performance of super
after the specs pass so I'll update it then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather do it eagerly if you don't mind.
I think in general we currently profile every condition unless it folds at PE time or known to be worthless to profile.
I can also do it when integrating the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure you can do it then - there's a couple of unprofiled branches in this method then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which other branches? restArgIndex
is PE constant.
Co-authored-by: Benoit Daloze <eregontp@gmail.com>
PullRequest: truffleruby/1994
Progress towards #2103.
Shopify#1.