Skip to content

Conversation

chrisseaton
Copy link
Collaborator

Progress towards #2103.

Shopify#1.


final Object[] restArgs;

if (restArg instanceof RubyArray) {
Copy link
Member

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)?

Copy link
Collaborator Author

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.

Copy link
Member

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.

Copy link
Collaborator Author

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.

Copy link
Member

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>
@eregon eregon self-assigned this Sep 19, 2020
@eregon eregon added this to the 20.3.0 milestone Sep 19, 2020
@eregon eregon added the in-ci The PR is being tested in CI. Do not push new commits. label Sep 19, 2020
graalvmbot pushed a commit that referenced this pull request Sep 23, 2020
PullRequest: truffleruby/1994
@graalvmbot graalvmbot merged commit c5eaf18 into oracle:master Sep 23, 2020
@chrisseaton chrisseaton deleted the super-spec branch October 1, 2020 12:41
@chrisseaton chrisseaton added the shopify Pull requests from Shopify label Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-ci The PR is being tested in CI. Do not push new commits. oca-signed shopify Pull requests from Shopify
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants