HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Thread-safe, in-memory LRU cache with a maximum item count of 10

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
itemmaximumwithcachethreadmemorysafecountlru

Problem

I'm attempting to master LRU Caching. It must be thread safe, and it should preferably perform as well as web image-cache, (avg ~1MB).

Please take a look to see if there's anything wrong, amiss, etc.

ViewController.m (requesting the cache):

- (void)viewWillAppear:(BOOL)animated {
    self.ricItems = [RicCache getCachedRicItems];
}

// -------------------------------------------------------

- (void)viewWillDisappear:(BOOL)animated {
    [RicCache cacheRicItems:self.ricItems];
    [super viewWillDisappear:animated];
}


The Caching Engine:

```
#import "RicCache.h"
#define kMenuStaleSeconds 10

@interface RicCache ()
+ (NSString *)cacheDirectory;
+ (NSString *)appVersion; // ...used when you update your app. Keep cache distinct per app version.
@end

@implementation RicCache
static NSMutableDictionary *memoryCache;
static NSMutableArray *recentlyAccessedKeys;
static int kCacheMemoryLimit;

+ (void)initialize {
NSString *cacheDirectory = [RicCache cacheDirectory];
if (![[NSFileManager defaultManager] fileExistsAtPath:cacheDirectory]){
[[NSFileManager defaultManager] createDirectoryAtPath:cacheDirectory
withIntermediateDirectories:YES
attributes:nil
error:nil];
}

// Invalidating the Cache.
// Check if app's current version is dated; if true, then clear it via 'clearCache':

double lastSavedCacheVersion = [[NSUserDefaults standardUserDefaults] doubleForKey:@"CACHE_VERSION"];
double currentAppVersion = [[RicCache appVersion] doubleValue];

if (lastSavedCacheVersion == 0.0f || lastSavedCacheVersion kCacheMemoryLimit) {
NSString *leastRecentlyUsedDataFilename = [recentlyAccessedKeys lastObject];
NSData *leastRecentlyUsedCacheData = [memoryCache objectForKey:leastRecentlyUsedDataFilename];
NSString *archivePath = [[RicCache cacheDirectory] strin

Solution

Here are some thoughts, quite unorganized.

-
You have put all functionality into class methods, and consequently the setup is
done in +initialize and the state kept in static variables. That makes it
impossible to have two independent caches (perhaps for independent parts of an
application). Also the memory cache can never deallocated (+dealloc
is never called).

I would suggest to alloc/init instances instead, e.g.

-(instancetype) initWithName:(NSString *)name;


would init a "cache manager" with storage in ../Caches/, and consequently
put the clean-up part into -dealloc, and make the state variables memoryCache etc instance variables.

-
Setting instance variables to nil in dealloc is not necessary, this
is done automatically by the ARC generated code.

-
Assuming that the app version is never 0.0, you can remove the first part
of the comparison

if (lastSavedCacheVersion == 0.0f || lastSavedCacheVersion < currentAppVersion) { ...


-
To retrieve the application version, it is not necessary to resort to CoreFoundation and juggle with CFStringRef and C-Strings. Simply:

NSDictionary *infoDict = [[NSBundle mainBundle] infoDictionary];
NSString *version = [infoDict objectForKey:(NSString *)kCFBundleVersionKey];


-
It is not necessary to check if an object is present in an array if
you want to remove it, i.e. you can simply remove the if-condition in

if ([recentlyAccessedKeys containsObject:fileName]){
    [recentlyAccessedKeys removeObject:fileName];
}


-
#define kMenuStaleSeconds 10 is nowhere used.

-
The methods

+ (void)cacheRicItems:(NSArray *)RicItems;
+ (NSMutableArray *)getCachedRicItems;


get/set one specific cached object (for the key "RicItems.archive"). I do not think they should implemented in the RicCache class at all.

If these are the only methods used by the view controller then the entire
caching makes no sense, because there always would be only one single object
in the memoryCache (for that key "RicItems.archive").

-
The dataForFile: method should be synchronized as well, because accessing
the memoryCache dictionary is not thread-safe.

-
Finally: Have a look at What advantage(s) does dispatch_sync have over @synchronized?. According to the bbums's answer given there synchronizing with dispatch_sync is faster than @synchronized. (I won't forget that because I gave a completely wrong answer :)

Code Snippets

-(instancetype) initWithName:(NSString *)name;
if (lastSavedCacheVersion == 0.0f || lastSavedCacheVersion < currentAppVersion) { ...
NSDictionary *infoDict = [[NSBundle mainBundle] infoDictionary];
NSString *version = [infoDict objectForKey:(NSString *)kCFBundleVersionKey];
if ([recentlyAccessedKeys containsObject:fileName]){
    [recentlyAccessedKeys removeObject:fileName];
}
+ (void)cacheRicItems:(NSArray *)RicItems;
+ (NSMutableArray *)getCachedRicItems;

Context

StackExchange Code Review Q#61237, answer score: 5

Revisions (0)

No revisions yet.