Skip to content
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

Merged
merged 2 commits into from Dec 14, 2016
Merged

Improve stack depth #4382

merged 2 commits into from Dec 14, 2016

Conversation

@headius
Copy link
Member

@headius headius commented Dec 13, 2016

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

headius added 2 commits Dec 13, 2016
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 #3810.
@headius headius added this to the JRuby 9.1.7.0 milestone Dec 13, 2016

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

This comment has been minimized.

@enebo

enebo Dec 13, 2016
Member

This simplification confuses me. Did we never reuse dyn scopes?

This comment has been minimized.

@headius

headius Dec 14, 2016
Author Member

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.

This comment has been minimized.

@enebo

enebo Dec 14, 2016
Member

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.

@kares
Copy link
Member

@kares kares commented Dec 14, 2016

+1 would be interested in the actual numbers you observed ... "nearly 50%" compared to which in #3810 :)

@headius
Copy link
Member Author

@headius headius commented Dec 14, 2016

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
JRuby master: 10399
JRuby 9.1.6.0 -X-C: 2969
JRuby master -X-C: 2924

I can get these longer-running numbers to be fairly consistent if I disable all background JIT in the JVM with -Xbatch. We are definitely beholden to the JIT when it comes to stack depth too.

@headius
Copy link
Member Author

@headius headius commented Dec 14, 2016

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.

@headius headius merged commit 7785320 into jruby:master Dec 14, 2016
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
@headius headius deleted the headius:improve-stack-depth branch Dec 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.