Caching Key Generation Considerations
Recently I was reviewing some code and ran across this – can you spot the problem?
var parameters = new List<SqlParameter>
{
new SqlParameter("@SomeID", someId),
new SqlParameter("@SomeOtherID", someOtherId),
new SqlParameter("@ServerDate", serverTime)
};
string cacheKey = string.Format("ResultSetName-sid{0}-soid{1}-date{2}",
parameters.Select(x => x.Value).ToArray());
var resultSet = Cache[cacheKey] as List<Foo>;
if (resultSet == null)
{
// do stuff and add resultSet to cache with cacheKey
}
There are actually a couple of different issues with this approach. The first one is that the serverTime parameter is literally sending in the current time (not date), which means that when it is .ToString()’d and joined into the cacheKey, it’s going to have information down to the second. Since this method is being called several times per second, that just means that there is going to be a new cache entry every second as a result of the choice of cache key. What’s worse, the old items won’t automatically be purged since there isn’t any 1-second absolute expiration here, so the ASP.NET Cache has to use its standard LRU algorithm to clean things up when it starts feeling memory pressure (which will probably happen pretty quickly). End result, a fair bit of needless thrashing and memory usage.
That’s the main problem, and in some performance tests using CacheManager I saw the total number of cache entries for the application reach up to six figures with this code in place. The simplest fix that retains the date parameter is to change the 3rd SqlParameter line to use serverTime.ToShortDateString() which ensures the key only varies by date and not by second, resulting in 86,399 fewer cache entries per day per other variation in the key.
The other question of course is why the date is in the key at all? If you really want the key to expire frequently, that’s what AbsoluteExpiration cache dependencies are for, and these will automatically clean up (or at least mark as ready to clean up) the expired entries, rather than just widowing their keys and leaving it up to the Cache’s memory management to take care of the mess. In general, dates make a poor choice for inclusion in cache keys, and raw datetimes are especially a bad idea.




Comments
Chris Marisic said on 25 Aug 2009 at 4:30 PM
Yeah I agree with you Steve that's some very poor usage of caching, you're probably worse off with that code there than if you didn't cache anything!
From seeing that snippit and being entirely disconnected from the problem that looks like it was a victim of premature optimization. That the developer was caching for the sake of caching without really thinking about what he was doing and why it would be better off doing it that way.