-
Notifications
You must be signed in to change notification settings - Fork 185
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
Calling method with splat arguments is slower than MRI #1595
Comments
Thanks for the report! It certainly looks like something odd is going on that we need to look into. As a minor quibble though, I think the method naming convention adopted in fast_ruby is misleading. I've raised this point and got buy in to go through and rename the methods. I just haven't found the time to do it yet. In the time since I originally filed the issue, MRI has gotten its own JIT and I expect to see the roles of "fast" and "slow" to change for some of these methods. All of this is a long-winded way of saying that we shouldn't necessarily have the same performance profile as MRI. In an ideal world, there'd be no measurable difference between the two forms of method calls, so you could then make the choice of one over the other by taste, rather than compromising for performance. And in that regard, we've missed the mark, even if we're 240x faster than MRI for the single array argument case. |
The interesting part in this benchmark, is whether |
Yeah. In MRI the splatted array is simply passed as a Ruby array (after enabling copy on write), so it doesn't actually copy any of the elements. Our argument passing code turns it into a Java |
MRI still seems to do at least one copy or walk, as it's >10x faster with As a note, the original benchmark code is also slightly incorrect because I'm also unsure of how representative is this benchmark. OTOH, optimizing splats by passing a copy-on-write Ruby Array would be a more general optimization. ITEMS = (1..10).to_a.freeze
def self.func(*args)
args << -1
end
p func(*ITEMS) # => [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, -1] So this is a long way for me to say I'd rather optimize this if it happens in real-world Ruby code than just a micro-benchmark for the sake of it. @gogainda Do you think splatting a large Array is a common pattern in Ruby? |
Don't think that this is common pattern, at least I haven't seen it in the wild. If you think that this benchmark doesn't represent real life application we can close this issue. |
I agree that handling smaller cases makes more sense. I suspect someone was looking at O(1) and O(n) and pumping the n value makes the resulting discrepancy easier to spot. |
Latest results (much better now, but still not comparable to mri implementation)
|
Hi there,
I was comparing the results from fast_ruby repo using Truffle Ruby vs MRI.
I notice that this code shows different results. I am not talking about instructions per seconds but difference between 'slow' and 'fast' methods. Ideally it should be the same as in MRI
vs
The text was updated successfully, but these errors were encountered: