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

Use putIfAbsent in CachedIntrospectionResults.addTypeDescriptor [SPR-12102] #16718

Closed
spring-projects-issues opened this issue Aug 19, 2014 · 5 comments
Assignees
Labels
type: enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Aug 19, 2014

Craig opened SPR-12102 and commented

In #16486, CachedIntrospectionResults was changed to use a ConcurrentHashMap for typeDescriptorCache. This change is definitely positive and had a significant improvement to performance in my testing.

I think that using putIfAbsent (instead of just put) would be a further improvement in the addTypeDescriptor method. Changing:

void addTypeDescriptor(PropertyDescriptor pd, TypeDescriptor td) {
     this.typeDescriptorCache.put(pd, td);
}

to

void addTypeDescriptor(PropertyDescriptor pd, TypeDescriptor td) {
     this.typeDescriptorCache.putIfAbsent(pd, td);
}

should result in less writes to volatile variables improve performance of getTypeDescriptor.

Changing the other ConcurrentHashMap (strongClassCache and softClassCache) put()'s to putIfAbsent()'s would probably also be beneficial.


Affects: 4.1 RC2

Issue Links:

  • #16486 Revisit class cache in CachedIntrospectionResults

Referenced from: commits af6ef5f, 781a6d2

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 19, 2014

Juergen Hoeller commented

Good point... Applied to all ConcurrentHashMaps in CachedIntrospectionResults for 4.1 GA!

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 19, 2014

Craig commented

Pull request: #629

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 19, 2014

Juergen Hoeller commented

Oops, I just noticed based on your pull request that my commit from a few minutes ago only works on JDK 8... where putIfAbsent has been added as a default method to the Map interface itself. Fixing this now towards ConcurrentMap declarations that work on JDK 6 and 7 as well.

As for using the pre-cached values as returned from putIfAbsent, this doesn't seem to make much difference, since the values are going to be equivalent anyway?

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 19, 2014

Craig commented

I figured that using the pre-cached values reduces the total number of objects in memory (by just a tiny amount) so it was worth doing. It doesn't seem like it should really matter, but I figured, why not do it as best as I could think of. :-)

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 19, 2014

Juergen Hoeller commented

Alright, along with fixing the ConcurrentMap declarations, I've changed the code to reuse existing cached values where possible (including TypeDescriptors in BeanWrapperImpl).

Juergen

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

No branches or pull requests

2 participants