Simplify the 'factory' MethodHandle signatures to drop 'dependency' types from them #1889
Conversation
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
![copybara-service[bot]](https://avatars.githubusercontent.com/in/44061?s=60&v=4){kind=small}
…ypes from them I thought modeling native types would be better but on reflection it just adds complexity for little gain. So turn all generated factories into `(InternalContext)->Object` types. This eliminates a bunch of `asType` adapters and should improve linkage performance a bit (not that this is a huge concern), but the major benefit is just simplicity. Performance is an interesting question here. In principle we are just adding 'no op' cast to `Object` on constructor and provides method invocations, and then `checkCast` instructions at constructor and method parameter injection points. So the waste is those extra `cast` operations. However, in the JVM those are nearly free (simple pointer check, can often be elided as part of normal invocation type checking). So my guess is that this is a no-op performance wise. Some limited benchmarking of `@Provides` and `@Inject` constructors bears this out. PiperOrigin-RevId: 740791709
copybara-service
bot
force-pushed
the
test_740497045
branch
from
8e2f099 to
17aa84c
Compare
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.
Simplify the 'factory' MethodHandle signatures to drop 'dependency' types from them
I thought modeling native types would be better but on reflection it just adds complexity for little gain. So turn all generated factories into
(InternalContext)->Objecttypes. This eliminates a bunch ofasTypeadapters and should improve linkage performance a bit (not that this is a huge concern), but the major benefit is just simplicity.Performance is an interesting question here. In principle we are just adding 'no op' cast to
Objecton constructor and provides method invocations, and thencheckCastinstructions at constructor and method parameter injection points. So the waste is those extracastoperations. However, in the JVM those are nearly free (simple pointer check, can often be elided as part of normal invocation type checking). So my guess is that this is a no-op performance wise. Some limited benchmarking of@Providesand@Injectconstructors bears this out.