Skip to content

respond_to?(<symbol>) => true optimization #8862

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

headius
Copy link
Member

@headius headius commented Jun 7, 2025

This optimizes the common case of respond_to? with a literal symbol. When the method is "naturally" respond_to? => true then that result can be cached behind the usual guards. This makes the success path essentially free.

The logic here has a lot of copied behavior from elsewhere and utilizes some now-static methods from InvokeSite, so it could be refactored along with the rest of indy logic to be much simpler.

@headius headius added this to the JRuby 10.0.1.0 milestone Jun 7, 2025
@headius
Copy link
Member Author

headius commented Jun 7, 2025

This could be expanded to make false results also constant by additionally checking if respond_to_missing? is the built-in version (which has no special logic). In that case, the method is "naturally" undefined, and without either it being defined or a respond_to_missing? being added, that would not change.

@headius headius force-pushed the respond_to_optz branch from c7c9dd6 to 939f4c1 Compare June 9, 2025 00:11
This optimizes the common case of respond_to? with a literal
symbol. When the method is "naturally" respond_to? => true then
that result can be cached behind the usual guards. This makes the
success path essentially free.

The logic here has a lot of copied behavior from elsewhere and
utilizes some now-static methods from InvokeSite, so it could be
refactored along with the rest of indy logic to be much simpler.

There's a lot of duplicated code here from other call site forms,
demonstrating the need for a refactor of all indy binding logic.
@headius headius force-pushed the respond_to_optz branch from 939f4c1 to d368cf1 Compare June 9, 2025 00:43
@headius
Copy link
Member Author

headius commented Jun 9, 2025

This needs some refactoring to work properly. I hack one fix in and break something else. On pause for the moment until I can get some time to rework indy call sites for more composability.

@headius headius marked this pull request as draft June 9, 2025 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant