Skip to content

Commit 17aa84c

Browse files
lukesandbergGuice Team
authored andcommitted
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)->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
1 parent b8dc884 commit 17aa84c

23 files changed

+208
-286
lines changed

core/src/com/google/inject/internal/BoundProviderFactory.java

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,13 @@ public T get(InternalContext context, Dependency<?> dependency, boolean linked)
6767
public MethodHandle getHandle(LinkageContext context, Dependency<?> dependency, boolean linked) {
6868
return context.makeHandle(
6969
this,
70-
dependency,
7170
() ->
7271
InternalMethodHandles.catchInternalProvisionExceptionAndRethrowWithSource(
73-
circularGetHandle(
74-
providerFactory.getHandle(context, PROVIDER_DEPENDENCY, /* linked= */ true),
75-
dependency,
76-
provisionCallback),
77-
providerKey)
78-
.asType(InternalMethodHandles.makeFactoryType(dependency)));
72+
circularGetHandle(
73+
providerFactory.getHandle(context, PROVIDER_DEPENDENCY, /* linked= */ true),
74+
dependency,
75+
provisionCallback),
76+
providerKey));
7977
}
8078

8179
@Override

core/src/com/google/inject/internal/ConstantFactory.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public Provider<T> makeProvider(InjectorImpl injector, Dependency<?> dependency)
5353
@Override
5454
public MethodHandle getHandle(
5555
LinkageContext context, Dependency<?> dependency, boolean linked) {
56-
var returnNull = MethodHandles.empty(InternalMethodHandles.makeFactoryType(dependency));
56+
var returnNull = InternalMethodHandles.constantFactoryGetHandle(null);
5757
if (dependency.isNullable()) {
5858
// Just return a constant null handle.
5959
return returnNull;
@@ -93,7 +93,7 @@ public Provider<T> makeProvider(InjectorImpl injector, Dependency<?> dependency)
9393

9494
@Override
9595
public MethodHandle getHandle(LinkageContext context, Dependency<?> dependency, boolean linked) {
96-
return InternalMethodHandles.constantFactoryGetHandle(dependency, instance);
96+
return InternalMethodHandles.constantFactoryGetHandle(instance);
9797
}
9898

9999
@Override

core/src/com/google/inject/internal/ConstantProviderInternalFactory.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,6 @@ public T get(InternalContext context, Dependency<?> dependency, boolean linked)
4747
@Override
4848
public MethodHandle getHandle(LinkageContext context, Dependency<?> dependency, boolean linked) {
4949
return circularGetHandleImmediate(
50-
InternalMethodHandles.constantFactoryGetHandle(jakarta.inject.Provider.class, provider),
51-
dependency,
52-
provisionCallback)
53-
.asType(InternalMethodHandles.makeFactoryType(dependency));
50+
InternalMethodHandles.constantFactoryGetHandle(provider), dependency, provisionCallback);
5451
}
5552
}

core/src/com/google/inject/internal/ConstructionProxy.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ interface ConstructionProxy<T> {
3838
/**
3939
* Returns the method handle for the constructor, using the supplied handles to provide
4040
* parameters.
41+
*
42+
* <p>The returned handle has {@link InternalMethodHandles#FACTORY_TYPE} as its signature.
4143
*/
4244
MethodHandle getConstructHandle(MethodHandle[] parameterHandles);
4345

core/src/com/google/inject/internal/ConstructorBindingImpl.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,6 @@ public MethodHandle getHandle(
322322
}
323323
return context.makeHandle(
324324
this,
325-
dependency,
326325
() -> constructorInjector.getConstructHandle(context, dependency, provisionCallback));
327326
}
328327

core/src/com/google/inject/internal/ConstructorInjector.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
package com.google.inject.internal;
1818

19-
import static com.google.inject.internal.InternalMethodHandles.castReturnToObject;
19+
import static com.google.common.base.Preconditions.checkState;
2020

2121
import com.google.common.collect.ImmutableSet;
2222
import com.google.inject.internal.ProvisionListenerStackCallback.ProvisionCallback;
@@ -102,6 +102,11 @@ MethodHandle getConstructHandle(
102102
var handle =
103103
constructionProxy.getConstructHandle(
104104
SingleParameterInjector.getAllHandles(linkageContext, parameterInjectors));
105+
checkState(
106+
handle.type().equals(InternalMethodHandles.FACTORY_TYPE),
107+
"expected %s but got %s",
108+
InternalMethodHandles.FACTORY_TYPE,
109+
handle.type());
105110
// If there are members injectors we call `finishConstructionAndSetReference` so that
106111
// cycle detection can find the newly constructed reference.
107112
if (membersInjector != null) {
@@ -116,7 +121,7 @@ MethodHandle getConstructHandle(
116121
membersHandle);
117122
// Then execute thje membersHandle after constructing the object (and calling
118123
// finishConstructionAndSetReference)
119-
handle = MethodHandles.foldArguments(membersHandle, castReturnToObject(handle));
124+
handle = MethodHandles.foldArguments(membersHandle, handle);
120125
} else {
121126
// Otherwise we are done!
122127
handle = InternalMethodHandles.finishConstruction(handle, circularFactoryId);

core/src/com/google/inject/internal/DefaultConstructionProxyFactory.java

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
package com.google.inject.internal;
1818

1919
import static com.google.inject.internal.InternalMethodHandles.castReturnTo;
20+
import static com.google.inject.internal.InternalMethodHandles.castReturnToObject;
2021
import static java.lang.invoke.MethodType.methodType;
21-
import static java.util.Arrays.stream;
2222

2323
import com.google.common.collect.ImmutableMap;
2424
import com.google.inject.spi.InjectionPoint;
@@ -133,20 +133,14 @@ public MethodHandle getConstructHandle(MethodHandle[] parameterHandles) {
133133
InternalMethodHandles.BIFUNCTION_APPLY_HANDLE
134134
.bindTo(fastConstructor)
135135
.asType(methodType(Object.class, Object.class, Object[].class));
136-
handle = MethodHandles.insertArguments(handle, 0, (Object) null);
136+
handle = MethodHandles.insertArguments(handle, 0, (Object) null); // no receiver type.
137137
handle = handle.asCollector(Object[].class, parameterHandles.length);
138-
handle =
139-
MethodHandles.filterArguments(
140-
handle,
141-
0,
142-
stream(parameterHandles)
143-
.map(InternalMethodHandles::castReturnToObject)
144-
.toArray(MethodHandle[]::new));
138+
// Pass all the parameters to the constructor.
139+
handle = MethodHandles.filterArguments(handle, 0, parameterHandles);
140+
// merge all the internalcontext parameters into a single object factory.
145141
handle =
146142
MethodHandles.permuteArguments(
147-
handle,
148-
methodType(Object.class, InternalContext.class),
149-
new int[parameterHandles.length]);
143+
handle, InternalMethodHandles.FACTORY_TYPE, new int[parameterHandles.length]);
150144
return handle;
151145
}
152146
}
@@ -197,15 +191,15 @@ public T newInstance(Object... arguments) throws InvocationTargetException {
197191
@Override
198192
public MethodHandle getConstructHandle(MethodHandle[] parameterHandles) {
199193
var type = target.type();
194+
// Adapt the parameter handles to the constructor signature.
200195
var typedHandles =
201196
IntStream.range(0, parameterHandles.length)
202197
.mapToObj(i -> castReturnTo(parameterHandles[i], type.parameterType(i)))
203198
.toArray(MethodHandle[]::new);
204199
var handle = MethodHandles.filterArguments(target, 0, typedHandles);
200+
handle = castReturnToObject(handle); // satisfy the signature of the factory type.
205201
return MethodHandles.permuteArguments(
206-
handle,
207-
methodType(type.returnType(), InternalContext.class),
208-
new int[typedHandles.length]);
202+
handle, InternalMethodHandles.FACTORY_TYPE, new int[typedHandles.length]);
209203
}
210204
}
211205
}

core/src/com/google/inject/internal/InitializableFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public T get(InternalContext context, Dependency<?> dependency, boolean linked)
4848

4949
@Override
5050
public MethodHandle getHandle(LinkageContext context, Dependency<?> dependency, boolean linked) {
51-
return InternalMethodHandles.initializableFactoryGetHandle(initializable, dependency);
51+
return InternalMethodHandles.initializableFactoryGetHandle(initializable);
5252
}
5353

5454
@Override

core/src/com/google/inject/internal/InjectorImpl.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -406,8 +406,7 @@ public Provider<Provider<T>> makeProvider(InjectorImpl injector, Dependency<?> d
406406
@Override
407407
public MethodHandle getHandle(
408408
LinkageContext context, Dependency<?> dependency, boolean linked) {
409-
return InternalMethodHandles.constantFactoryGetHandle(
410-
Provider.class, providedBinding.getProvider());
409+
return InternalMethodHandles.constantFactoryGetHandle(providedBinding.getProvider());
411410
}
412411
};
413412
}

core/src/com/google/inject/internal/InjectorShell.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ public Provider<Injector> makeProvider(InjectorImpl injector, Dependency<?> depe
286286
@Override
287287
public MethodHandle getHandle(
288288
LinkageContext context, Dependency<?> dependency, boolean linked) {
289-
return InternalMethodHandles.constantFactoryGetHandle(dependency, injector);
289+
return InternalMethodHandles.constantFactoryGetHandle(injector);
290290
}
291291

292292
@Override
@@ -335,7 +335,7 @@ public Provider<Logger> makeProvider(InjectorImpl injector, Dependency<?> depend
335335
@Override
336336
public MethodHandle getHandle(
337337
LinkageContext context, Dependency<?> dependency, boolean linked) {
338-
return InternalMethodHandles.constantFactoryGetHandle(dependency, makeLogger(dependency));
338+
return InternalMethodHandles.constantFactoryGetHandle(makeLogger(dependency));
339339
}
340340

341341
private static Logger makeLogger(Dependency<?> dependency) {

core/src/com/google/inject/internal/InternalFactory.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,7 @@ default Provider<T> makeProvider(InjectorImpl injector, Dependency<?> dependency
6060
default MethodHandle getHandle(LinkageContext context, Dependency<?> dependency, boolean linked) {
6161
// The default implementation simply delegates to the `get` method.
6262
MethodHandle handle = InternalMethodHandles.INTERNAL_FACTORY_GET_HANDLE.bindTo(this);
63-
return MethodHandles.insertArguments(handle, 1, dependency, linked)
64-
.asType(InternalMethodHandles.makeFactoryType(dependency));
63+
return MethodHandles.insertArguments(handle, 1, dependency, linked);
6564
}
6665

6766
/**

core/src/com/google/inject/internal/InternalFactoryToScopedProviderAdapter.java

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717
package com.google.inject.internal;
1818

1919
import static com.google.common.base.Preconditions.checkNotNull;
20-
import static com.google.inject.internal.InternalMethodHandles.castReturnTo;
21-
import static com.google.inject.internal.InternalMethodHandles.castReturnToObject;
2220
import static com.google.inject.internal.InternalMethodHandles.findVirtualOrDie;
2321
import static java.lang.invoke.MethodType.methodType;
2422

@@ -98,7 +96,7 @@ public MethodHandle getHandle(LinkageContext context, Dependency<?> dependency,
9896
MethodHandles.foldArguments(
9997
invokeProvider,
10098
MethodHandles.insertArguments(INTERNAL_CONTEXT_SET_DEPENDENCY_HANDLE, 1, dependency));
101-
return castReturnTo(invokeProvider, dependency.getKey().getTypeLiteral().getRawType());
99+
return invokeProvider;
102100
}
103101

104102
private static final MethodHandle INTERNAL_CONTEXT_SET_DEPENDENCY_HANDLE =
@@ -166,8 +164,11 @@ public MethodHandle getHandle(
166164

167165
private static MethodHandle getHandleForConstant(
168166
Dependency<?> dependency, Object source, Object value) {
169-
var handle = InternalMethodHandles.constantFactoryGetHandle(dependency, value);
170-
handle = InternalMethodHandles.nullCheckResult(handle, source, dependency);
167+
var handle = InternalMethodHandles.constantFactoryGetHandle(value);
168+
// Since this is a constant, we only need to null check if it is actually null.
169+
if (value == null) {
170+
handle = InternalMethodHandles.nullCheckResult(handle, source, dependency);
171+
}
171172
return handle;
172173
}
173174

@@ -195,14 +196,12 @@ static final class SingletonCallSite extends MutableCallSite {
195196
// This will allow us to eventually 'fold' the result into the callsite.
196197
// (InternalContext, InternalContext) -> Object
197198
var invokeBootstrap =
198-
MethodHandles.filterArguments(
199-
BOOTSTRAP_CALL_MH.bindTo(this), 1, castReturnToObject(actualGetHandle));
200-
201-
// (InternalContext, InternalContext) -> T
202-
invokeBootstrap = castReturnTo(invokeBootstrap, actualGetHandle.type().returnType());
203-
// (InternalContext) -> T
199+
MethodHandles.filterArguments(BOOTSTRAP_CALL_MH.bindTo(this), 1, actualGetHandle);
200+
// Merge the two internalContext parameters into one.
201+
// (InternalContext) -> Object
204202
invokeBootstrap =
205-
MethodHandles.permuteArguments(invokeBootstrap, actualGetHandle.type(), new int[2]);
203+
MethodHandles.permuteArguments(
204+
invokeBootstrap, InternalMethodHandles.FACTORY_TYPE, new int[2]);
206205
setTarget(invokeBootstrap);
207206
}
208207

0 commit comments

Comments
 (0)