Encode new and old name in aliased calls #7702
Merged
Conversation
This file contains 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
headius
force-pushed
the
timmy_down_the_well
branch
2 times, most recently
from
71942d1
to
393c685
Compare
JRuby only pushes the actual method name on the stack, since up until recently it was the only name needed. However the when the `__callee__` method was added, the aliased name also became necessary. Passing another argument along the dynamic call stack would take considerable work, which might happen along with making the call path flexible enough to handle allocation-free keyword arguments. For now, however this hack may be good enough. The logic here encodes the aliased name and the original name in the same string passed along the call stack. Since this name is used in very few places – making super calls, setting up the interpreter backtrace, and the `__method__` and `__callee__` methods – only those locations need to be modified to receive the encoded string. The format of the encoded string uses a leading null character, followed by the new name, another null character, and the old name. This allows an easy check for encoding using charAt(0), and the individual names can then be pulled out easily. The null character is also very unlikely to show up in any real method identifier (and may actually be illegal). Because the two name are encoded into a single string, it is necessary to allocate a new string to extract either. However, the places where these names are used already have significant overhead, this allocation may not be a big concern. This change is also hopefully temporary until we can better thread this data through the dynamic call stack. Fixes jruby#2305
headius
force-pushed
the
timmy_down_the_well
branch
from
393c685
to
25e92f8
Compare
This was referenced Mar 2, 2023
With __callee__ working these modifications are no longer needed.
Any existing consumers of getFrameName would be expecting to get the current method's name (as for super) so we should not break that expectation. Places that need to pass through the composite name, such as from frame to frame or binding, should use getCompositeName and handle decoding the component names.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
JRuby only pushes the actual method name on the stack, since up until recently it was the only name needed. However the when the
__callee__method was added, the aliased name also became necessary.Passing another argument along the dynamic call stack would take considerable work, which might happen along with making the call path flexible enough to handle allocation-free keyword arguments. For now, however this hack may be good enough.
The logic here encodes the aliased name and the original name in the same string passed along the call stack. Since this name is used in very few places – making super calls, setting up the interpreter backtrace, and the
__method__and__callee__methods – only those locations need to be modified to receive the encoded string.The format of the encoded string uses a leading null character, followed by the new name, another null character, and the old name. This allows an easy check for encoding using charAt(0), and the individual names can then be pulled out easily. The null character is also very unlikely to show up in any real method identifier (and may actually be illegal).
Because the two name are encoded into a single string, it is necessary to allocate a new string to extract either. However, the places where these names are used already have significant overhead, this allocation may not be a big concern.
This change is also hopefully temporary until we can better thread this data through the dynamic call stack.
Fixes #2305