Skip to content

Don't initialize Arguments::name twice (VM part) #2201

Closed
wants to merge 1 commit into from

1 participant

@ryoqun
Rubinius member
ryoqun commented Mar 13, 2013

InlineCache::execute always initializes Arguments::name to appropriate name. So,
there is no need to initialize Arguments::name when Arguments objects are
constructed and immediately handed to it.

This commit is only for the VM. One for the JIT will follow.

@ryoqun ryoqun Don't initialize Arguments::name twice (VM part)
InlineCache::execute always initializes Arguments::name to appropriate name. So,
there is no need to initialize Arguments::name when Arguments objects are
constructed and immediately handed to it.

This commit is only for the VM. One for the JIT will follow.
b8d94f8
@ryoqun
Rubinius member
ryoqun commented Mar 27, 2013

As discuessed below, I'll close this pull request at the moment:

[23:21] ryoqun I found that that pull request is't so effective as touted. GCC seems to generate bad code without the patch affecting perfomance. Clang aren't affected. So, I think this is GCC-specific issue.
[23:21] rockyb Speaking of issues, and info on rocky/rbx-trepanning#5?
[23:22] rockyb This is really a rubinius bug.
[23:22] ryoqun So, I think I need to investigate why GCC performs bad. But I haven't
[23:23] ryoqun And this is reproducible on other machines, etc.
[23:25] dbussink ryoqun: ah ok, that's weird with gcc then yeah
[23:26] ryoqun yeah. so, it's a low priority one in my todo list..
[23:27] ryoqun if it's bother you, I can close the pull request.
[23:28] dbussink ryoqun: yeah, well, if it's not really actionable at this point it's probably better to close it
[23:28] dbussink so other people also don't think, huh, what's this for
[23:29] ryoqun kk

@ryoqun ryoqun closed this Mar 27, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.