Profiling query methods for mapping model

[noter]

I have some time to discover profiler features so I don't want to waste it, so I have played a little bit with spring-data-mongo.

I find that some query to mapping model (after warmup) are little bit uneffective because it always use reflection call or whole collection iteration for example.

So I made some changes to cache this methods call and I gained ~ 20% reduction in query time.

If someone confirm my assumptions, I can make clean patch for this.

Test: org.springframework.data.mongodb.performance.PerformanceTests#readAfterWarmup

[odrotbohm]

I'd definitely like to see this folded into the actual implementations. It seems that some of the improvements could be made in Spring Data Commons types directly (e.g. holding a name -> property map in BasicPersistentEntity instead of a plain Set we always need to iterate over). The same applies to PreferredConstructor.isConstructorParameter(…).

[odrotbohm]

I've just pushed the changes you made applicable for Spring Data Commons as fix for the ticket I created. With that in place the performance gap is close to gone (roughly seeing <0.1% difference). So I wondered whether it's really necessary to apply the rather wild patch to CustomConversions. What do you think?

The current state should be available in https://github.com/SpringSource/spring-data-mongodb/tree/performance-%2337.

[noter]

Yeah sure, I done this CustomConversions changes before I found problem core point, I think.

[noter]

Or not :)
I looked at it again and I'm still getting ~5% diff.
Here is a printscreen from profiler that show the time consumed by CustomConversions in org.springframework.data.mongodb.core.convert.MappingMongoConverter.read call compared to ProfiledCustomConversions
So now 16% of whole "read" call is CustomConversions.hasCustomReadTarget

[noter]

Ah I reamember, change are wild because this is the fastest way to cache with two dimensions (two parameters) without creating custom Map key. Creating key with two params every time to get something from cache are taking same time as it was without cache.
ConcurrentHashMap can't have null key so there is Foo placeholder.

[odrotbohm]

Okay, then let's consider it. I already had renamed your Foo class to Placeholder as it essentially seemed to indicate "already looked at but nothing found". Is that correct? What's the reason for the CacheHolder? Cant we just keep a Map<Class<?>, Map<Class<?>, Class<?>>>?

Finally, what tool did you use to profile the code? I tried jvisualvm but it was not delivering any results comparable to what you got.

[noter]

Yes, all this is because I use ConcurrentHashMap where we can't use null key or value.
CacheHolder is for the same reason. getCustomReadTarget can return null but we can't keep it in ConcurrentHashMap, to prevent every time checking.

I use yourkit java profiler with CPU tracing.

But didn't you get any diff in readAfterWarmup test. First time run I get 1% but I run tast (from maven console) 20x and I get avg 5%, so i think it is not accident :)

[odrotbohm]

Applied in fixes for both DATACMNS-332 and DATAMONGO-682. Thanks for spotting that stuff and doing the heavy lifting.