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

Spring-context optimization: LinkedMultiValueMap and ArrayList should be initialized with specified capacity for preventing collections from resizing [SPR-17079] #21616

Closed
spring-projects-issues opened this issue Jul 24, 2018 · 2 comments
Assignees
Labels
in: core type: enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Jul 24, 2018

Alexander Fedorov opened SPR-17079 and commented

Spring-context library should be tuned in memory allocation point of view.
We use JMC and JFR API for aggregating memory allocation events.
Minor GC could be tuned with help of investigating of memory allocation events inside new TLAB.
Major GC could be tuned with help of investigating of memory allocation events outside TLAB.
We focused on reducing Minor GC overhead.

Following Setup Categories used:

spring.interceptor.resolveCaches org.springframework.cache.interceptor.AbstractCacheResolver.resolveCaches()
spring.interceptor.CacheAspectSupport org.springframework.cache.interceptor.CacheAspectSupport.execute()
spring.expression org.springframework.expression.spel.CodeFlow
spring.core org.springframework.core.BridgeMethodResolver.findBridgedMethod()

logger.info org.slf4j.impl.Log4jLoggerAdapter.info()
logger.info weblogic.i18n.logging.NonCatalogLogger.info()

logger.warning weblogic.i18n.logging.NonCatalogLogger.warning()
spring.beans org.springframework.beans.factory.support.DefaultListableBeanFactory.getBean()
spring.aop org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.<allocate inside tlab>()
spring.aop org.springframework.aop.aspectj.MethodInvocationProceedingJoinPoint.<allocate inside tlab>()
spring.aop org.springframework.aop.aspectj.RuntimeTestWalker$ThisInstanceOfResidueTestVisitor.<allocate inside tlab>()
  1. AbstractCacheResolver.resolveCaches

!screenshot-1.png|thumbnail!


package org.springframework.cache.interceptor;


public abstract class AbstractCacheResolver implements CacheResolver, InitializingBean {

	@Override
	public Collection<? extends Cache> resolveCaches(CacheOperationInvocationContext<?> context) {
		Collection<String> cacheNames = getCacheNames(context);
		if (cacheNames == null) {
			return Collections.emptyList();
		}
		else {
			Collection<Cache> result = new ArrayList<Cache>();
			for (String cacheName : cacheNames) {
				Cache cache = this.cacheManager.getCache(cacheName);
				if (cache == null) {
					throw new IllegalArgumentException("Cannot find cache named '" +
							cacheName + "' for " + context.getOperation());
				}
				result.add(cache);
			}
			return result;
		}
	}

Here a screenshot of diff between 4.1.x and 5.0.x versions:

!screenshot-2.png|thumbnail!

Need specify ArrayList capacity right in creation.

  1. CacheAspectSupport$CacheOperationContexts.init()

!screenshot-4.png|thumbnail!

package org.springframework.cache.interceptor;

public abstract class CacheAspectSupport extends AbstractCacheInvoker
		implements InitializingBean, SmartInitializingSingleton, ApplicationContextAware {


	private class CacheOperationContexts {

		private final MultiValueMap<Class<? extends CacheOperation>, CacheOperationContext> contexts =
				new LinkedMultiValueMap<Class<? extends CacheOperation>, CacheOperationContext>();

		public CacheOperationContexts(Collection<? extends CacheOperation> operations, Method method,
				Object[] args, Object target, Class<?> targetClass) {

			for (CacheOperation operation : operations) {
				this.contexts.add(operation.getClass(), getOperationContext(operation, method, args, target, targetClass));
			}
		}

		public Collection<CacheOperationContext> get(Class<? extends CacheOperation> operationClass) {
			Collection<CacheOperationContext> result = this.contexts.get(operationClass);
			return (result != null ? result : Collections.<CacheOperationContext>emptyList());
		}
	}

Here a screenshot of diff between 4.1.x and 5.0.x versions:
!screenshot-3.png|thumbnail!

Should be added capacity for Map. it help reduce resize and unnecessary memory allocation.

Possible solutions:

	private class CacheOperationContexts {

		private final MultiValueMap<Class<? extends CacheOperation>, CacheOperationContext> contexts;// =
				//new LinkedMultiValueMap<Class<? extends CacheOperation>, CacheOperationContext>();

		public CacheOperationContexts(Collection<? extends CacheOperation> operations, Method method,
				Object[] args, Object target, Class<?> targetClass) {
			contexts = new LinkedMultiValueMap<Class<? extends CacheOperation>, CacheOperationContext>( (int) Math.round( operations.size()/0.75) );
			for (CacheOperation operation : operations) {
				this.contexts.add(operation.getClass(), getOperationContext(operation, method, args, target, targetClass));
			}
		}

		public Collection<CacheOperationContext> get(Class<? extends CacheOperation> operationClass) {
			Collection<CacheOperationContext> result = (contexts != null? this.contexts.get(operationClass): null );
			return (result != null ? result : Collections.<CacheOperationContext>emptyList());
		}
	}


	@Override
	public Collection<? extends Cache> resolveCaches(CacheOperationInvocationContext<?> context) {
		Collection<String> cacheNames = getCacheNames(context);
		if (cacheNames == null) {
			return Collections.emptyList();
		}
		else {
			Collection<Cache> result = new ArrayList<Cache>( (int) Math.round(cacheNames.size() ) );
			for (String cacheName : cacheNames) {
				Cache cache = this.cacheManager.getCache(cacheName);
				if (cache == null) {
					throw new IllegalArgumentException("Cannot find cache named '" +
							cacheName + "' for " + context.getOperation());
				}
				result.add(cache);
			}
			return result;
		}
	}

Affects: 5.0.7

Attachments:

Issue Links:

  • #21642 Initialize pre-filled HashMaps with large enough capacity (e.g. in HttpMethod)
@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 24, 2018

Juergen Hoeller commented

Good catch! Fixed for 5.1 now, and to be backported to 5.0.8 as well.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jul 25, 2018

Alexander Fedorov commented

Hi Juergen Hoeller,
sorry for Math.round here:

Collection<Cache> result = new ArrayList<Cache>( (int) Math.round(cacheNames.size() ) );

should be just

Collection<Cache> result = new ArrayList<Cache>( cacheNames.size() );

P.S. my copy-paste anti-pattern

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants