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

@Cacheable annotation lead to a huge memory allocation from the side of OperationCache equals/hashCode [SPR-16490] #21033

Closed
spring-projects-issues opened this issue Feb 13, 2018 · 3 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: duplicate A duplicate of another issue type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Feb 13, 2018

Alexander Fedorov opened SPR-16490 and commented

Hello Team,
we have faced a very intensive CPU consuming. One of the reason is very high GC activity.
GC activity could be splitted into two points:

  1. minor GC collections which should be done very quickly in parallel
  2. major GC collections ( FullGC ) which should be a rare case and should not lead to a long delay from app ( because FullGC is stop the world in our case ).

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 use JMC and JFR API for aggregating memory allocation events.
It looks like

  1. Memory allocation events inside new TLAB
    !allocation_inside_TLAB.jpg|thumbnail!

  2. Memory allocation events outside TLAB
    !allocation_outside_TLAB.jpg|thumbnail!

Where higlighted categories are:

spring.cache_interceptor  org.springframework.cache.interceptor
spring.TransactionInterceptor.invoke org.springframework.transaction.interceptor.TransactionInterceptor.invoke()

Hotspots looks like:

!allocation_inside_TLAB_hotspots.jpg|thumbnail!

Issue is here:

!decompiled.jpg|thumbnail!

protected StringBuilder getOperationDescription()
 {
   StringBuilder result = new StringBuilder(getClass().getSimpleName());
   result.append("[").append(this.name);
   result.append("] caches=").append(this.cacheNames);
   result.append(" | key='").append(this.key);
   result.append("' | keyGenerator='").append(this.keyGenerator);
   result.append("' | cacheManager='").append(this.cacheManager);
   result.append("' | cacheResolver='").append(this.cacheResolver);
   result.append("' | condition='").append(this.condition).append("'");
   return result;
 }

Our version is

Manifest-Version: 1.0
Created-By: 1.8.0_20 (Oracle Corporation)
Implementation-Title: spring-context
Implementation-Version: 4.1.3.RELEASE

But issue actual for latest version too.

I suggest that it could be implemented as below:

@Override
	public String toString() {
		if( toStringCachedResult == null ) {
			toStringCachedResult = getOperationDescription().toString().intern();
		}
		return toStringCachedResult;
	}

	@Override
	public boolean equals(Object o) {
		if (this == o) return true;
		if (o == null || getClass() != o.getClass()) return false;

		CacheOperation that = (CacheOperation) o;

		return toString().equals(that.toString());

	}



	@Override
	public int hashCode() {
		if(hashCodeCachedResult == 0 ) {
			hashCodeCachedResult = toString().hashCode();
		}
		return hashCodeCachedResult;
	}

	private int hashCodeCachedResult = 0;
	private String toStringCachedResult = null;

	/**
	 * Return an identifying description for this caching operation.
	 * <p>Available to subclasses, for inclusion in their {@code toString()} result.
	 */
	protected StringBuilder getOperationDescription() {
		StringBuilder result = new StringBuilder(getClass().getSimpleName());
		result./*append("[").*/append(this.name);
		result./*append("] caches=").*/append(this.cacheNames);
		result./*append(" | key='").*/append(this.key);
		result./*append("' | keyGenerator='").*/append(this.keyGenerator);
		result./*append("' | cacheManager='").*/append(this.cacheManager);
		result./*append("' | cacheResolver='").*/append(this.cacheResolver);
		result./*append("' | condition='").*/append(this.condition)/*.append("'")*/;
		return result;
	}

Main idea is caching and interning string objects for minimizing memory allocation from the side of method equals and compare int instead of String for hashCode.

Our cusomized version of [^CacheOperation.java] is attached.

Thank you!


Affects: 4.3.14

Attachments:

Issue Links:

@spring-projects-issues
Copy link
Collaborator Author

Alexander Fedorov commented

I will prepare pull request.
In progress..

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Feb 13, 2018

Juergen Hoeller commented

This looks like a duplicate of #18542 and #20549 which have been addressed in the 4.3.x line. So while your report looks entirely reasonable against 4.1.x, please double-check against 4.3.14 where no such patching should be necessary.

@spring-projects-issues
Copy link
Collaborator Author

Alexander Fedorov commented

Hi Juergen Hoeller,

we found that 4.3.14 successfully fixed issue. So ticket could be closed as Duplicate.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants