To cache or not to cache - GetCustomAttributes

I just had a scenario where GetCustomAttributes turned out to be the performance bottleneck. In my case it was getting called hundreds of thousands of times in a dataset with many rows and this made the problem easy to isolate. Caching the attributes solved the problem.

Preliminary testing led to a barely noticeable performance hit at about 5000 calls on a modern day machine. (And it became drastically more noticeable as the dataset size increased.)

I generally agree with the other answers about premature optimization, however, on a scale of CPU instruction to DB call, I'd suggest that GetCustomAttributes would lean more towards the latter.

The only way you can know for sure, is to profile it. I am sorry if this sounds like a cliche. But the reason why a saying is a cliche is often because it's true.

Caching the attribute is actually making the code more complex, and more error prone. So you might want to take this into account-- your development time-- before you decide.

So like optimization, don't do it unless you have to.

From my experience ( I am talking about AutoCAD-like Windows Application, with a lot of click-edit GUI operations and heavy number crunching), the reading of custom attribute is never--even once-- the performance bottleneck.

Your question is a case of premature optimization.

You don't know the inner workings of the reflection classes and therefore are making assumptions about the performance implications of calling GetCustomAttributes multiple times. The method itself could well cache its output already, meaning your code would actually add overhead with no performance improvement.

Save your brain cycles for thinking about things which you already know are problems!

You will get better bangs for your bucks if you replace the body of your method with this:

return Attribute.GetCustomAttribute(Member, AttributeType,false); // only look in the current member and don't go up the inheritance tree.

If you really need to cache on a type-basis:

public static class MyCacheFor<T>
{
    static MyCacheFor()
    {
        // grab the data
        Value = ExtractExpensiveData(typeof(T));
    }

    public static readonly MyExpensiveToExtractData Value;

    private static MyExpensiveToExtractData ExtractExpensiveData(Type type)
    {
        // ...
    }
}

Beats dictionary lookups everytime. Plus it's threadsafe:)

Cheers, Florian

PS: Depends how often you call this. I had some cases where doing a lot of serialization using reflection really called for caching, as usual, you want to measure the performance gain versus the memory usage increase. Instrument your memory use and profile your CPU time.