Also increment thresholds in indy sites for method_missing. #4602
Conversation
Fixes #4596. JRuby currently does not cache anything for method_missing calls (i.e. calls to methods that do not exist on the target class's method table). Instead, after discovering the missing method, it proceeds down a slow lookup path, dispatching to method_missing with a symbolic method name. However this logic still attempts to acquire a SwitchPoint from the target class, since that SP must be acquired before the initial method lookup to ensure proper invalidation ordering. Normally, this isn't a problem. If the class is still relatively static, the SwitchPoint will be created once, and then the cost of calling method_missing is the same as for non-indy. However for type very rapidly. This led to a large number of unused SwitchPoint being created, one for each class. And since the method_missing path never cached anything, it never updated call site thresholds, resulting in call sites that would continue this logic forever. Normal calls, if they fail repeatedly, will eventually fail the entire site and revert to non-indy method dispatch (currently a simple monomorphic cache). This patch lifts the threshold-bumping out of the caching logic so it can also be used by method_missing dispatches. This allows repeated method_missing calls against different classes to also fail the site, reverting it to non-indy behavior. With this patch, the benchmark in #4596 runs faster with indy than without, as expected.
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.
Fixes #4596.
JRuby currently does not cache anything for method_missing calls
(i.e. calls to methods that do not exist on the target class's
method table). Instead, after discovering the missing method, it
proceeds down a slow lookup path, dispatching to method_missing
with a symbolic method name.
However this logic still attempts to acquire a SwitchPoint from
the target class, since that SP must be acquired before the
initial method lookup to ensure proper invalidation ordering.
Normally, this isn't a problem. If the class is still relatively
static, the SwitchPoint will be created once, and then the cost
of calling method_missing is the same as for non-indy. However for
type very rapidly. This led to a large number of unused
SwitchPoint being created, one for each class. And since the
method_missing path never cached anything, it never updated call
site thresholds, resulting in call sites that would continue this
logic forever.
Normal calls, if they fail repeatedly, will eventually fail the
entire site and revert to non-indy method dispatch (currently a
simple monomorphic cache).
This patch lifts the threshold-bumping out of the caching logic
so it can also be used by method_missing dispatches. This allows
repeated method_missing calls against different classes to also
fail the site, reverting it to non-indy behavior. With this patch,
the benchmark in #4596 runs faster with indy than without, as
expected.