-
-
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
Improve stack depth #4382
Improve stack depth #4382
Conversation
This also removes some complication from CompiledIRMethod that related to calling methods that have no built-in call protocol. Only metaclass bodies do not have call protocol, so methods do not need to check for it every time.
Improves stack depth for jruby#3810.
|
||
public CompiledIRMetaClassBody(MethodHandle handle, IRScope scope, RubyModule implementationClass) { | ||
super(handle, scope, Visibility.PUBLIC, implementationClass, scope.receivesKeywordArgs()); | ||
|
||
boolean reuseParentDynScope = scope.getFlags().contains(IRFlags.REUSE_PARENT_DYNSCOPE); |
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.
This simplification confuses me. Did we never reuse dyn scopes?
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.
This class is only used for metaclass bodies, which never reuse their parent scope. I realized this and removed that logic, and the rest boiled away.
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.
yeah I guess that is probably true of any hard scope. Almost makes me think we should probably ask scope if it needs a scope or not instead of interpreting our flags per its runtime object (method, block). Anyways, thanks for the explanation.
+1 would be interested in the actual numbers you observed ... "nearly 50%" compared to which in #3810 :) |
On the simple recursion benchmark, I got anywhere from 1800 to 2000 without the patch and 2500-2600 with it. That's not quite 50% but it's pretty good. The variability may be due to the JVM's background jit swapping in more efficient versions of the methods on the stack earlier or later. Another side effect of JIT. A version of the bench that runs ten times, reporting the last stack depth, gives the following numbers JRuby 9.1.6.0: 5671 I can get these longer-running numbers to be fairly consistent if I disable all background JIT in the JVM with |
Note that these only improve the stack depth of calling Ruby methods; it does not do anything specific to improve calls to native Java code or through Java integration, block/proc dispatches through yield or call, or reflected invocations like send and method.call. There's more work to do. |
These commits add more call paths through a few of our DynamicMethod subclasses for Ruby methods, to reduce the number of stack frames they consume. This improves JIT stack depth for the examples in #3810 by nearly 50%, with a somewhat smaller gain for the interpreter (which has more frames).