Skip to content

Commit

Permalink
Introduce ProxiedInterfacesCache for JdkDynamicAopProxy
Browse files Browse the repository at this point in the history 
Closes gh-30499
  • Loading branch information
@jhoeller
jhoeller committed Dec 22, 2023
1 parent cd8bc2f commit 44c652e
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 73 deletions.
44 changes: 30 additions & 14 deletions spring-aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,23 @@ public class AdvisedSupport extends ProxyConfig implements Advised {
*/
private List<Advisor> advisors = new ArrayList<>();

/**
* List of minimal {@link AdvisorKeyEntry} instances,
* to be assigned to the {@link #advisors} field on reduction.
* @since 6.0.10
* @see #reduceToAdvisorKey
*/
private List<Advisor> advisorKey = this.advisors;

/**
* Optional field for {@link AopProxy} implementations to store metadata in.
* Used for {@link JdkDynamicAopProxy.ProxiedInterfacesCache}.
* @since 6.1.3
* @see JdkDynamicAopProxy#JdkDynamicAopProxy(AdvisedSupport)
*/
@Nullable
transient Object proxyMetadataCache;


/**
* No-arg constructor for use as a JavaBean.
Expand Down Expand Up @@ -491,6 +506,7 @@ public List<Object> getInterceptorsAndDynamicInterceptionAdvice(Method method, @
*/
protected void adviceChanged() {
this.methodCache.clear();
this.proxyMetadataCache = null;
}

/**
Expand Down Expand Up @@ -551,18 +567,6 @@ Object getAdvisorKey() {
}


//---------------------------------------------------------------------
// Serialization support
//---------------------------------------------------------------------

private void readObject(ObjectInputStream ois) throws IOException, ClassNotFoundException {
// Rely on default serialization; just initialize state after deserialization.
ois.defaultReadObject();

// Initialize transient fields.
this.methodCache = new ConcurrentHashMap<>(32);
}

@Override
public String toProxyConfigString() {
return toString();
Expand All @@ -584,6 +588,19 @@ public String toString() {
}


//---------------------------------------------------------------------
// Serialization support
//---------------------------------------------------------------------

private void readObject(ObjectInputStream ois) throws IOException, ClassNotFoundException {
// Rely on default serialization; just initialize state after deserialization.
ois.defaultReadObject();

// Initialize transient fields.
this.methodCache = new ConcurrentHashMap<>(32);
}


/**
* Simple wrapper class around a Method. Used as the key when
* caching methods, for efficient equals and hashCode comparisons.
Expand Down Expand Up @@ -633,7 +650,7 @@ public int compareTo(MethodCacheKey other) {
* @see #getConfigurationOnlyCopy()
* @see #getAdvisorKey()
*/
private static class AdvisorKeyEntry implements Advisor {
private static final class AdvisorKeyEntry implements Advisor {

private final Class<?> adviceType;

Expand All @@ -643,7 +660,6 @@ private static class AdvisorKeyEntry implements Advisor {
@Nullable
private final String methodMatcherKey;


public AdvisorKeyEntry(Advisor advisor) {
this.adviceType = advisor.getAdvice().getClass();
if (advisor instanceof PointcutAdvisor pointcutAdvisor) {
Expand Down
118 changes: 69 additions & 49 deletions spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package org.springframework.aop.framework;

import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.Serializable;
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
Expand Down Expand Up @@ -71,34 +73,16 @@ final class JdkDynamicAopProxy implements AopProxy, InvocationHandler, Serializa
private static final long serialVersionUID = 5531744639992436476L;


/*
* NOTE: We could avoid the code duplication between this class and the CGLIB
* proxies by refactoring "invoke" into a template method. However, this approach
* adds at least 10% performance overhead versus a copy-paste solution, so we sacrifice
* elegance for performance (we have a good test suite to ensure that the different
* proxies behave the same :-)).
* This way, we can also more easily take advantage of minor optimizations in each class.
*/
private static final String COROUTINES_FLOW_CLASS_NAME = "kotlinx.coroutines.flow.Flow";

/** We use a static Log to avoid serialization issues. */
private static final Log logger = LogFactory.getLog(JdkDynamicAopProxy.class);

private static final String COROUTINES_FLOW_CLASS_NAME = "kotlinx.coroutines.flow.Flow";

/** Config used to configure this proxy. */
private final AdvisedSupport advised;

private final Class<?>[] proxiedInterfaces;

/**
* Is the {@link #equals} method defined on the proxied interfaces?
*/
private boolean equalsDefined;

/**
* Is the {@link #hashCode} method defined on the proxied interfaces?
*/
private boolean hashCodeDefined;
/** Cached in {@link AdvisedSupport#proxyMetadataCache}. */
private transient ProxiedInterfacesCache cache;


/**
Expand All @@ -110,8 +94,17 @@ final class JdkDynamicAopProxy implements AopProxy, InvocationHandler, Serializa
public JdkDynamicAopProxy(AdvisedSupport config) throws AopConfigException {
Assert.notNull(config, "AdvisedSupport must not be null");
this.advised = config;
this.proxiedInterfaces = AopProxyUtils.completeProxiedInterfaces(this.advised, true);
findDefinedEqualsAndHashCodeMethods(this.proxiedInterfaces);

// Initialize ProxiedInterfacesCache if not cached already
ProxiedInterfacesCache cache;
if (config.proxyMetadataCache instanceof ProxiedInterfacesCache proxiedInterfacesCache) {
cache = proxiedInterfacesCache;
}
else {
cache = new ProxiedInterfacesCache(config);
config.proxyMetadataCache = cache;
}
this.cache = cache;
}


Expand All @@ -125,13 +118,13 @@ public Object getProxy(@Nullable ClassLoader classLoader) {
if (logger.isTraceEnabled()) {
logger.trace("Creating JDK dynamic proxy: " + this.advised.getTargetSource());
}
return Proxy.newProxyInstance(determineClassLoader(classLoader), this.proxiedInterfaces, this);
return Proxy.newProxyInstance(determineClassLoader(classLoader), this.cache.proxiedInterfaces, this);
}

@SuppressWarnings("deprecation")
@Override
public Class<?> getProxyClass(@Nullable ClassLoader classLoader) {
return Proxy.getProxyClass(determineClassLoader(classLoader), this.proxiedInterfaces);
return Proxy.getProxyClass(determineClassLoader(classLoader), this.cache.proxiedInterfaces);
}

/**
Expand Down Expand Up @@ -160,28 +153,6 @@ private ClassLoader determineClassLoader(@Nullable ClassLoader classLoader) {
return classLoader;
}

/**
* Finds any {@link #equals} or {@link #hashCode} method that may be defined
* on the supplied set of interfaces.
* @param proxiedInterfaces the interfaces to introspect
*/
private void findDefinedEqualsAndHashCodeMethods(Class<?>[] proxiedInterfaces) {
for (Class<?> proxiedInterface : proxiedInterfaces) {
Method[] methods = proxiedInterface.getDeclaredMethods();
for (Method method : methods) {
if (AopUtils.isEqualsMethod(method)) {
this.equalsDefined = true;
}
if (AopUtils.isHashCodeMethod(method)) {
this.hashCodeDefined = true;
}
if (this.equalsDefined && this.hashCodeDefined) {
return;
}
}
}
}


/**
* Implementation of {@code InvocationHandler.invoke}.
Expand All @@ -198,11 +169,11 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
Object target = null;

try {
if (!this.equalsDefined && AopUtils.isEqualsMethod(method)) {
if (!this.cache.equalsDefined && AopUtils.isEqualsMethod(method)) {
// The target does not implement the equals(Object) method itself.
return equals(args[0]);
}
else if (!this.hashCodeDefined && AopUtils.isHashCodeMethod(method)) {
else if (!this.cache.hashCodeDefined && AopUtils.isHashCodeMethod(method)) {
// The target does not implement the hashCode() method itself.
return hashCode();
}
Expand Down Expand Up @@ -324,4 +295,53 @@ public int hashCode() {
return JdkDynamicAopProxy.class.hashCode() * 13 + this.advised.getTargetSource().hashCode();
}


//---------------------------------------------------------------------
// Serialization support
//---------------------------------------------------------------------

private void readObject(ObjectInputStream ois) throws IOException, ClassNotFoundException {
// Rely on default serialization; just initialize state after deserialization.
ois.defaultReadObject();

// Initialize transient fields.
this.cache = new ProxiedInterfacesCache(this.advised);
}


/**
* Holder for the complete proxied interfaces and derived metadata,
* to be cached in {@link AdvisedSupport#proxyMetadataCache}.
* @since 6.1.3
*/
static final class ProxiedInterfacesCache {

Class<?>[] proxiedInterfaces;

boolean equalsDefined;

boolean hashCodeDefined;

ProxiedInterfacesCache(AdvisedSupport config) {
this.proxiedInterfaces = AopProxyUtils.completeProxiedInterfaces(config, true);

// Find any {@link #equals} or {@link #hashCode} method that may be defined
//on the supplied set of interfaces.
for (Class<?> proxiedInterface : this.proxiedInterfaces) {
Method[] methods = proxiedInterface.getDeclaredMethods();
for (Method method : methods) {
if (AopUtils.isEqualsMethod(method)) {
this.equalsDefined = true;
}
if (AopUtils.isHashCodeMethod(method)) {
this.hashCodeDefined = true;
}
if (this.equalsDefined && this.hashCodeDefined) {
return;
}
}
}
}
}

}
21 changes: 11 additions & 10 deletions spring-context/src/test/java/org/springframework/aop/framework/JdkDynamicProxyTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ protected AopProxy createAopProxy(AdvisedSupport as) {

@Test
void testNullConfig() {
assertThatIllegalArgumentException().isThrownBy(() ->
new JdkDynamicAopProxy(null));
assertThatIllegalArgumentException().isThrownBy(() -> new JdkDynamicAopProxy(null));
}

@Test
Expand All @@ -69,10 +68,8 @@ void testProxyIsJustInterface() {
JdkDynamicAopProxy aop = new JdkDynamicAopProxy(pc);

Object proxy = aop.getProxy();
boolean condition = proxy instanceof ITestBean;
assertThat(condition).isTrue();
boolean condition1 = proxy instanceof TestBean;
assertThat(condition1).isFalse();
assertThat(proxy instanceof ITestBean).isTrue();
assertThat(proxy instanceof TestBean).isFalse();
}

@Test
Expand Down Expand Up @@ -131,11 +128,15 @@ void testProxyNotWrappedIfIncompatible() {

@Test
void testEqualsAndHashCodeDefined() {
AdvisedSupport as = new AdvisedSupport(Named.class);
as.setTarget(new Person());
JdkDynamicAopProxy aopProxy = new JdkDynamicAopProxy(as);
Named proxy = (Named) aopProxy.getProxy();
Named named = new Person();
AdvisedSupport as = new AdvisedSupport(Named.class);
as.setTarget(named);

Named proxy = (Named) new JdkDynamicAopProxy(as).getProxy();
assertThat(proxy).isEqualTo(named);
assertThat(named.hashCode()).isEqualTo(proxy.hashCode());

proxy = (Named) new JdkDynamicAopProxy(as).getProxy();
assertThat(proxy).isEqualTo(named);
assertThat(named.hashCode()).isEqualTo(proxy.hashCode());
}
Expand Down

0 comments on commit 44c652e

Please sign in to comment.