Separate varargs and specific-arity names in JIT. Fixes #4482#4498
Merged
headius merged 2 commits intojruby:masterfrom Feb 22, 2017
Merged
Separate varargs and specific-arity names in JIT. Fixes #4482#4498headius merged 2 commits intojruby:masterfrom
headius merged 2 commits intojruby:masterfrom
Conversation
Originally, varargs and specific-arity paths were emitted in toto as their own JVM method bodies. With recent changes, the varargs path now calls the specific-arity path, causing an extra synthetic entry in the JVM stack trace. This confounded the backtrace miner, since it had no marker with which to omit the synthetic version, and so Kernel#caller and friends contained an additional invalid entry. This in turn caused require_relative to fail to find the caller's path, resulting in a nil path value passed to File.realpath, which triggered this bug. It likely only affected invokedynamic because our indy logic must be calling no-arg methods via the varargs path (a separate issue to be fixed). My modification here explicitly separates the synthetic varargs name from the specific-arity name by adding a known varargs marker that can be filtered out when mining the backtrace. This is a big hacky; the state being managed by the jit is very ad-hoc and prone to bugs. It will need some additional cleanup after 9.1.8.0.
Member
Author
|
The additional "issue" I mentioned is a non-issue for now. Indy will always call through the varargs path the first time a call site is bound, since it only has a single varargs binding path in InvokeSite. Fixing the issue would require having separate bootstrap logic for each arity, and would only save us the varargs array and overhead on the first (re)bind of a given call site. Since most indy sites will eventually settle into one or more direct variable OR specific paths, this is not necessary. However, we may want to fix it for sites that fail to bind, such as those that have too many targets or that are rebound too many times. |
Previously, indirect indy calls would always box arguments, which affects failed sites, Java sites, and others. This fixes jruby#4499 by setting up fail paths for arities up to 3 arguments, same as the non-indy call paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Originally, varargs and specific-arity paths were emitted in toto
as their own JVM method bodies. With recent changes, the varargs
path now calls the specific-arity path, causing an extra
synthetic entry in the JVM stack trace. This confounded the
backtrace miner, since it had no marker with which to omit the
synthetic version, and so Kernel#caller and friends contained an
additional invalid entry. This in turn caused require_relative to
fail to find the caller's path, resulting in a nil path value
passed to File.realpath, which triggered this bug.
It likely only affected invokedynamic because our indy logic must
be calling no-arg methods via the varargs path (a separate issue
to be fixed).
My modification here explicitly separates the synthetic varargs
name from the specific-arity name by adding a known varargs marker
that can be filtered out when mining the backtrace.
This is a big hacky; the state being managed by the jit is very
ad-hoc and prone to bugs. It will need some additional cleanup
after 9.1.8.0.
See #4482.