search

chriseldredge / Lucene.Net.Linq

LINQ provider to run native queries on a Lucene.Net index
Other
151 stars 66 forks source link

Performance gain from caching of GetProperties #92

Open khalidsalomao opened 9 years ago

khalidsalomao commented 9 years ago

There is a performance gain from caching GetProperties result.

Since the type properties are immutable we can safely save it in a static property for ReflectionDocumentMapper<T>.

A simple benchmark indicates a performance gain of 5-10x (depends on the number of properties a class have).

Here is the code that you can open in LinqPad and test it! https://gist.github.com/khalidsalomao/7f3a6c3f6c514f584d6a

@chriseldredge, when I was review ReflectionDocumentMapper<T> I noticed that the GetProperties only return instance properties. What do you think of also supporting its parent public properties by passing the flag BindingFlags.FlattenHierarchy?

chriseldredge commented 8 years ago

Since there are additional reflection calls in BuildFieldMap and BuildFieldKeyMap, does it make sense to cache the entire instance of ReflectionDocumentMapper<T> for reuse? Perhaps LuceneDataProvider should keep an instance cache of T to ReflectionDocumentMapper<T>?

Regarding BindingFlags.FlattenHierarchy, it probably makes sense to do that. I suppose it could break existing projects if they're not expecting it, but it seems unlikely.

khalidsalomao commented 8 years ago

Sure, caching ReflectionDocumentMapper<T> in LuceneDataProvider will be better. I will take a look into that!

Regarding BindingFlags.FlattenHierarchy, it was my mistake. Best keep it as it is! After a while, I remembered that it affects only static members... Sorry about that!