-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
using explicit return with external reference from define_method is 66x slower on 9k #3715
Comments
Updated numbers with
|
Still happens on jruby master (102a088) |
I have a pretty good idea on what the issue is. A slightly less good idea on how it should be fixed. The highest level description of the problem is that we cannot know at the time we generate the IR for this block whether it is a generic block or whether it is a method definition. In a generic block, return has extra potential behavior like in this snippet: def foo
[1].each { |e| retiurn e }
end As a result of this scenario we generate extra IR instructions to check for this case. That extra checking is what is killing performance. Here is the IR for the two versions: Slow
fast:
As for solutions...When there is no capture at all we completely rewrite the block as if it was a method. This is crazy fast but we did not do the extra work to make this work with read-only captures. If we could then this particular case would also be super fast. We have no infrastructure to detect read-only captures so I am not sure how much work it would be to solve it this way. A second possible solution would be to rebuild the source since IRClosure contains the original source for the block. We could assume the return in this case will only fall out of the method definition and it would be as fast as the 'fast' version. I added a 'super fast' to represent our potential if we put in the extra lambda hoisting logic we need for the former solution vs minimal rewriting in the latter solution: require 'benchmark/ips'
class Slow
m = :meh?
define_method(m) do
return m
end
end
class SuperFast
define_method(:meh?) do
:meh?
end
end
class Fast
m = :meh?
define_method(m) do
m
end
end
sf = SuperFast.new
f = Fast.new
s = Slow.new
Benchmark.ips do |x|
x.config(:time => 6, :warmup => 3)
x.report("super fast") { sf.meh? }
x.report("fast") { f.meh? }
x.report("slow") { s.meh? }
x.compare!
end
|
in jruby 9k using explicit return in a define_method block makes the code execution extremely slow due to its inability to optimize the code see more: jruby/jruby#3715
@headius actually bothered to profile our existing logic and found a gaping regression in 2066af4. This ups performance to nearly non-return:
I am also working on rebuilding the block in the presence of define_method so we can strip out unneeded instrs. Once that is done we should see slow match fast. |
Great stuff! thanks for explanations & details, extremely useful learning! |
Awesome feedback @enebo, thanks! Since the commit brings jruby back to previous performance levels, I'll close this issue. |
We'll leave this open until @enebo is happy with his |
Yeah I want this on the radar since I believe I can have this particular test case scream |
Deferring this one. Most of the perf issue has been fixed but my attempts at rebuilding closures on the fly made me realize we need to internally refactor some aspects of IRs lifecycle again. Nearly everything I tried tripped over something we assumed we would not do. |
@enebo Up to you if you want to try to do more on this for 9.1.3.0. If not, punt to 9.1.4.0 or 9.2. |
I was hoping to land the inlining work which is also grappling with lifecycle issues. I did not get it done though. Once it is done then converting this live will be trivial. Kicking down the road a bit longer. |
The original reported issue no longer seems to be a problem. The "slow" version is still slower than the "fast" version, but less than 2x.
@enebo You can reopen this if you want, but perhaps open a different bug if you still want to have something open for the remaining perf difference. |
Environment
Darwin Joaos-MBP.lan 15.3.0 Darwin Kernel Version 15.3.0: Thu Dec 10 18:40:58 PST 2015; root:xnu-3248.30.4~1/RELEASE_X86_64 x86_64
java version "1.8.0_20"
Java(TM) SE Runtime Environment (build 1.8.0_20-b26)
Java HotSpot(TM) 64-Bit Server VM (build 25.20-b23, mixed mode)
Testing the following code:
Expected Behavior
I expect the slowness of the Slow class to be consistent across ruby implementations
Actual Behavior
The text was updated successfully, but these errors were encountered: