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

Update ShadowWrangler to iterate through methods using getDeclaredMethods #8941

Closed
wants to merge 0 commits into from

Conversation

hoisie
Copy link
Contributor

@hoisie hoisie commented Mar 27, 2024

Update ShadowWrangler to iterate through methods using getDeclaredMethods

Previously, in ShadowWrangler, shadow method lookup was performed using
ShadowClass.findDeclaredMethod. It was called once to look for an exact match
of a shadow method, and sometimes called again to check for a looseSignatures
match.

There are plans to add new features and capabilities to the way that shadow
methods are matched. For example:

  • looseSignatures being replaced with a more minimal
    @classname("internal.type") annotation.
  • If the signature of a method changes across SDK levels, we could introduce
    different method names that map to the same method name.

However, to search for methods that cannot be matched using
ShadowClass.findDeclaredMethod, it is required to iterate over all candidate
methods using ShadowClass.findDeclaredMethods.

There were some questions about the performance of using
ShadowClass.findDeclaredMethods + iteration. However, after some preliminary
benchmarks, this approach is surprisingly approximately 25% faster than using
ShadowClass.findDeclaredMethod. It is perhaps due to the internal caching of
ShadowClass.findDeclaredMethods.

With this change, it will be possible to perform more advanced filtering and
searching for methods.

For #8841

@hoisie
Copy link
Contributor Author

hoisie commented Mar 27, 2024

FYI @utzcoz I updated ShadowWrangler to use ShadowClass.getDeclaredMethods and iterate over the methods. Surprisingly it's like 25% faster than calling getDeclaredMethod. So I think using this approach, we can do things like @ClassName for loose signatures, and your method name mapping PR.

@utzcoz
Copy link
Member

utzcoz commented Mar 27, 2024

@hoisie Cool. I can wait your change landing.

@copybara-service copybara-service bot changed the title Update ShadowWrangler to iterate through methods Update ShadowWrangler to iterate through methods using getDeclaredMethods Mar 28, 2024
@copybara-service copybara-service bot closed this Mar 28, 2024
@copybara-service copybara-service bot deleted the piper_619288002 branch March 28, 2024 17:22
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.

None yet

2 participants