rework method interception so that methods that aren't intercepted don't go through cglib, reducing the number of stack frames in most method calls. this has a slight side effect that additional proxy classes are generated for a single class if (and only if) the intercepted methods change. if the intercepted methods remain the same, then the proxy classes will continue to be shared (so things like assistedinject will not blow up the heap).
git-svn-id: https://google-guice.googlecode.com/svn/trunk@1482 d779f126-a31b-0410-b53b-1d3aecad763e
diff --git a/core/src/com/google/inject/internal/ProxyFactory.java b/core/src/com/google/inject/internal/ProxyFactory.java
index 74965e3..28020b2 100644
--- a/core/src/com/google/inject/internal/ProxyFactory.java
+++ b/core/src/com/google/inject/internal/ProxyFactory.java
@@ -46,15 +46,6 @@
*/
final class ProxyFactory<T> implements ConstructionProxyFactory<T> {
- private static final net.sf.cglib.proxy.MethodInterceptor NO_OP_METHOD_INTERCEPTOR
- = new net.sf.cglib.proxy.MethodInterceptor() {
- public Object intercept(
- Object proxy, Method method, Object[] arguments, MethodProxy methodProxy)
- throws Throwable {
- return methodProxy.invokeSuper(proxy, arguments);
- }
- };
-
private final InjectionPoint injectionPoint;
private final ImmutableMap<Method, List<MethodInterceptor>> interceptors;
private final Class<T> declaringClass;
@@ -124,7 +115,7 @@
MethodInterceptorsPair pair = methodInterceptorsPairs.get(i);
if (!pair.hasInterceptors()) {
- callbacks[i] = NO_OP_METHOD_INTERCEPTOR;
+ callbacks[i] = net.sf.cglib.proxy.NoOp.INSTANCE;
continue;
}
@@ -154,8 +145,14 @@
}
@SuppressWarnings("unchecked")
- Class<? extends Callback>[] callbackTypes = new Class[methods.size()];
- Arrays.fill(callbackTypes, net.sf.cglib.proxy.MethodInterceptor.class);
+ Class<? extends Callback>[] callbackTypes = new Class[callbacks.length];
+ for (int i = 0; i < callbacks.length; i++) {
+ if (callbacks[i] == net.sf.cglib.proxy.NoOp.INSTANCE) {
+ callbackTypes[i] = net.sf.cglib.proxy.NoOp.class;
+ } else {
+ callbackTypes[i] = net.sf.cglib.proxy.MethodInterceptor.class;
+ }
+ }
// Create the proxied class. We're careful to ensure that all enhancer state is not-specific
// to this injector. Otherwise, the proxies for each injector will waste PermGen memory
diff --git a/core/test/com/google/inject/MethodInterceptionTest.java b/core/test/com/google/inject/MethodInterceptionTest.java
index 27c5db4..d236ea4 100644
--- a/core/test/com/google/inject/MethodInterceptionTest.java
+++ b/core/test/com/google/inject/MethodInterceptionTest.java
@@ -23,6 +23,7 @@
import static com.google.inject.matcher.Matchers.only;
import com.google.inject.spi.ConstructorBinding;
import java.lang.reflect.Method;
+import java.util.Arrays;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
@@ -49,6 +50,12 @@
return null;
}
}
+
+ private final class NoOpInterceptor implements MethodInterceptor {
+ public Object invoke(MethodInvocation methodInvocation) throws Throwable {
+ return methodInvocation.proceed();
+ }
+ }
public void testSharedProxyClasses() {
Injector injector = Guice.createInjector(new AbstractModule() {
@@ -58,30 +65,27 @@
}
});
- Injector nullFoosInjector = injector.createChildInjector(new AbstractModule() {
+ Injector childOne = injector.createChildInjector(new AbstractModule() {
protected void configure() {
bind(Interceptable.class);
}
});
- Interceptable nullFoos = nullFoosInjector.getInstance(Interceptable.class);
- assertNotNull(nullFoos.bar());
- assertNull(nullFoos.foo());
+ Interceptable nullFoosOne = childOne.getInstance(Interceptable.class);
+ assertNotNull(nullFoosOne.bar());
+ assertNull(nullFoosOne.foo());
- Injector nullFoosAndBarsInjector = injector.createChildInjector(new AbstractModule() {
+ Injector childTwo = injector.createChildInjector(new AbstractModule() {
protected void configure() {
bind(Interceptable.class);
- bindInterceptor(Matchers.any(), Matchers.returns(only(Bar.class)),
- new ReturnNullInterceptor());
}
});
- Interceptable bothNull = nullFoosAndBarsInjector.getInstance(Interceptable.class);
- assertNull(bothNull.bar());
- assertNull(bothNull.foo());
+ Interceptable nullFoosTwo = childTwo.getInstance(Interceptable.class);
+ assertNull(nullFoosTwo.foo());
assertSame("Child injectors should share proxy classes, otherwise memory leaks!",
- nullFoos.getClass(), bothNull.getClass());
+ nullFoosOne.getClass(), nullFoosTwo.getClass());
}
public void testGetThis() {
@@ -174,18 +178,54 @@
assertEquals("testInterceptedMethodThrows", stackTraceElement[3].getMethodName());
}
}
+
+ public void testNotInterceptedMethodsInInterceptedClassDontAddFrames() {
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ protected void configure() {
+ bindInterceptor(Matchers.any(), Matchers.returns(only(Foo.class)),
+ new NoOpInterceptor());
+ }
+ });
+
+ Interceptable interceptable = injector.getInstance(Interceptable.class);
+ assertNull(interceptable.lastElements);
+ interceptable.foo();
+ boolean cglibFound = false;
+ for (int i = 0; i < interceptable.lastElements.length; i++) {
+ if (interceptable.lastElements[i].toString().contains("cglib")) {
+ cglibFound = true;
+ break;
+ }
+ }
+ assertTrue(Arrays.asList(interceptable.lastElements).toString(), cglibFound);
+ cglibFound = false;
+
+ interceptable.bar();
+ for (int i = 0; i < interceptable.lastElements.length; i++) {
+ if (interceptable.lastElements[i].toString().contains("cglib")) {
+ cglibFound = true;
+ break;
+ }
+ }
+ assertFalse(Arrays.asList(interceptable.lastElements).toString(), cglibFound);
+ }
static class Foo {}
static class Bar {}
public static class Interceptable {
+ StackTraceElement[] lastElements;
+
public Foo foo() {
+ lastElements = Thread.currentThread().getStackTrace();
return new Foo() {};
}
public Bar bar() {
+ lastElements = Thread.currentThread().getStackTrace();
return new Bar() {};
}
public String explode() throws Exception {
+ lastElements = Thread.currentThread().getStackTrace();
throw new Exception("kaboom!");
}
}
