From 0c61b49f0aca41f29ea886d310a5ae851944bbe3 Mon Sep 17 00:00:00 2001 From: Nick Lockwood Date: Tue, 14 Jul 2015 04:53:54 -0700 Subject: [PATCH] Improved RCTCache performance + fixed border color crash Summary: RCTCache had really bad insertion performance when the cache was full due to having to LRU-sort the entries. This was making color animations very slow. I've fixed this in two ways: 1) by removing the sort and doing a linear search to remove old entries, which changes insertion perf to O(n) in the worst case instead of O(n log n) or even (n2). 2) by reducing the size of the color cache to 128 from 1024, which should be fine for normal use, without penalising animation performance. Separately, border colors were not being retained, which caused crashes when the color cache was cleared. I've fixed that by retaining the border colors inside RCTView. --- React/Base/RCTCache.h | 7 +- React/Base/RCTCache.m | 162 +++++++++++++++++++++++++--------------- React/Base/RCTConvert.m | 2 +- React/Views/RCTView.h | 2 +- React/Views/RCTView.m | 60 ++++++++------- 5 files changed, 145 insertions(+), 88 deletions(-) diff --git a/React/Base/RCTCache.h b/React/Base/RCTCache.h index 9a4bef4df..67e26c3b7 100644 --- a/React/Base/RCTCache.h +++ b/React/Base/RCTCache.h @@ -15,7 +15,7 @@ * outside of the specified cost/count limits, and will be automatically * cleared in the event of a memory warning. */ -@interface RCTCache : NSCache +@interface RCTCache : NSCache /** * The total number of objects currently resident in the cache. @@ -33,6 +33,11 @@ - (id)objectForKeyedSubscript:(id)key; - (void)setObject:(id)obj forKeyedSubscript:(id)key; +/** + * Enumerate cached objects + */ +- (void)enumerateKeysAndObjectsUsingBlock:(void (^)(id key, id obj, BOOL *stop))block; + @end @protocol RCTCacheDelegate diff --git a/React/Base/RCTCache.m b/React/Base/RCTCache.m index 073e3faaa..a0646a401 100644 --- a/React/Base/RCTCache.m +++ b/React/Base/RCTCache.m @@ -28,15 +28,6 @@ @implementation RCTCacheEntry -+ (instancetype)entryWithObject:(id)object cost:(NSUInteger)cost sequenceNumber:(NSInteger)sequenceNumber -{ - RCTCacheEntry *entry = [[self alloc] init]; - entry.object = object; - entry.cost = cost; - entry.sequenceNumber = sequenceNumber; - return entry; -} - @end @interface RCTCache_Private : NSObject @@ -46,24 +37,28 @@ @property (nonatomic, assign) NSUInteger totalCostLimit; @property (nonatomic, copy) NSString *name; -@property (nonatomic, assign) NSUInteger totalCost; @property (nonatomic, strong) NSMutableDictionary *cache; -@property (nonatomic, assign) BOOL delegateRespondsToWillEvictObject; -@property (nonatomic, assign) BOOL delegateRespondsToShouldEvictObject; -@property (nonatomic, assign) BOOL currentlyCleaning; +@property (nonatomic, assign) NSUInteger totalCost; @property (nonatomic, assign) NSInteger sequenceNumber; -@property (nonatomic, strong) NSLock *lock; @end @implementation RCTCache_Private +{ + BOOL _delegateRespondsToWillEvictObject; + BOOL _delegateRespondsToShouldEvictObject; + BOOL _currentlyCleaning; + NSMutableArray *_entryPool; + NSLock *_lock; +} -- (instancetype)init +- (id)init { if ((self = [super init])) { //create storage _cache = [[NSMutableDictionary alloc] init]; + _entryPool = [[NSMutableArray alloc] init]; _lock = [[NSLock alloc] init]; _totalCost = 0; @@ -95,7 +90,7 @@ [_lock lock]; _countLimit = countLimit; [_lock unlock]; - [self cleanUp]; + [self cleanUp:NO]; } - (void)setTotalCostLimit:(NSUInteger)totalCostLimit @@ -103,7 +98,7 @@ [_lock lock]; _totalCostLimit = totalCostLimit; [_lock unlock]; - [self cleanUp]; + [self cleanUp:NO]; } - (NSUInteger)count @@ -111,40 +106,51 @@ return [_cache count]; } -- (void)cleanUp +- (void)cleanUp:(BOOL)keepEntries { [_lock lock]; - NSUInteger maxCount = [self countLimit] ?: INT_MAX; - NSUInteger maxCost = [self totalCostLimit] ?: INT_MAX; - NSUInteger totalCount = [_cache count]; - if (totalCount > maxCount || _totalCost > maxCost) + NSUInteger maxCount = _countLimit ?: INT_MAX; + NSUInteger maxCost = _totalCostLimit ?: INT_MAX; + NSUInteger totalCount = _cache.count; + NSMutableArray *keys = [_cache.allKeys mutableCopy]; + while (totalCount > maxCount || _totalCost > maxCost) { - //sort, oldest first - NSArray *keys = [[_cache allKeys] sortedArrayUsingComparator:^NSComparisonResult(id key1, id key2) { - RCTCacheEntry *entry1 = self.cache[key1]; - RCTCacheEntry *entry2 = self.cache[key2]; - return (NSComparisonResult)MIN(1, MAX(-1, entry1.sequenceNumber - entry2.sequenceNumber)); - }]; + NSInteger lowestSequenceNumber = INT_MAX; + RCTCacheEntry *lowestEntry = nil; + id lowestKey = nil; //remove oldest items until within limit for (id key in keys) { - if (totalCount <= maxCount && _totalCost <= maxCost) - { - break; - } RCTCacheEntry *entry = _cache[key]; - if (!_delegateRespondsToShouldEvictObject || [self.delegate cache:(RCTCache *)self shouldEvictObject:entry]) + if (entry.sequenceNumber < lowestSequenceNumber) + { + lowestSequenceNumber = entry.sequenceNumber; + lowestEntry = entry; + lowestKey = key; + } + } + + if (lowestKey) + { + [keys removeObject:lowestKey]; + if (!_delegateRespondsToShouldEvictObject || + [_delegate cache:(RCTCache *)self shouldEvictObject:lowestEntry.object]) { if (_delegateRespondsToWillEvictObject) { _currentlyCleaning = YES; - [self.delegate cache:(RCTCache *)self willEvictObject:entry]; + [self.delegate cache:(RCTCache *)self willEvictObject:lowestEntry.object]; _currentlyCleaning = NO; } - [_cache removeObjectForKey:key]; - _totalCost -= entry.cost; + [_cache removeObjectForKey:lowestKey]; + _totalCost -= lowestEntry.cost; totalCount --; + if (keepEntries) + { + [_entryPool addObject:lowestEntry]; + lowestEntry.object = nil; + } } } } @@ -161,8 +167,8 @@ { //sort, oldest first (in case we want to use that information in our eviction test) keys = [keys sortedArrayUsingComparator:^NSComparisonResult(id key1, id key2) { - RCTCacheEntry *entry1 = self.cache[key1]; - RCTCacheEntry *entry2 = self.cache[key2]; + RCTCacheEntry *entry1 = self->_cache[key1]; + RCTCacheEntry *entry2 = self->_cache[key2]; return (NSComparisonResult)MIN(1, MAX(-1, entry1.sequenceNumber - entry2.sequenceNumber)); }]; } @@ -171,12 +177,12 @@ for (id key in keys) { RCTCacheEntry *entry = _cache[key]; - if (!_delegateRespondsToShouldEvictObject || [self.delegate cache:(RCTCache *)self shouldEvictObject:entry]) + if (!_delegateRespondsToShouldEvictObject || [_delegate cache:(RCTCache *)self shouldEvictObject:entry.object]) { if (_delegateRespondsToWillEvictObject) { _currentlyCleaning = YES; - [self.delegate cache:(RCTCache *)self willEvictObject:entry]; + [_delegate cache:(RCTCache *)self willEvictObject:entry.object]; _currentlyCleaning = NO; } [_cache removeObjectForKey:key]; @@ -208,7 +214,7 @@ } } -- (id)objectForKey:(id)key +- (id)objectForKey:(id)key { [_lock lock]; RCTCacheEntry *entry = _cache[key]; @@ -227,7 +233,7 @@ return [self objectForKey:key]; } -- (void)setObject:(id)obj forKey:(id)key +- (void)setObject:(id)obj forKey:(id)key { [self setObject:obj forKey:key cost:0]; } @@ -237,27 +243,44 @@ [self setObject:obj forKey:key cost:0]; } -- (void)setObject:(id)obj forKey:(id)key cost:(NSUInteger)g +- (void)setObject:(id)obj forKey:(id)key cost:(NSUInteger)g { + if (!obj) + { + [self removeObjectForKey:key]; + return; + } RCTAssert(!_currentlyCleaning, @"It is not possible to modify cache from within the implementation of this delegate method."); [_lock lock]; _totalCost -= [_cache[key] cost]; _totalCost += g; - _cache[key] = [RCTCacheEntry entryWithObject:obj cost:g sequenceNumber:_sequenceNumber++]; + RCTCacheEntry *entry = _cache[key]; + if (!entry) { + entry = [[RCTCacheEntry alloc] init]; + _cache[key] = entry; + } + entry.object = obj; + entry.cost = g; + entry.sequenceNumber = _sequenceNumber++; if (_sequenceNumber < 0) { [self resequence]; } [_lock unlock]; - [self cleanUp]; + [self cleanUp:YES]; } -- (void)removeObjectForKey:(id)key +- (void)removeObjectForKey:(id)key { RCTAssert(!_currentlyCleaning, @"It is not possible to modify cache from within the implementation of this delegate method."); [_lock lock]; - _totalCost -= [_cache[key] cost]; - [_cache removeObjectForKey:key]; + RCTCacheEntry *entry = _cache[key]; + if (entry) { + _totalCost -= entry.cost; + entry.object = nil; + [_entryPool addObject:entry]; + [_cache removeObjectForKey:key]; + } [_lock unlock]; } @@ -267,20 +290,42 @@ [_lock lock]; _totalCost = 0; _sequenceNumber = 0; + for (RCTCacheEntry *entry in _cache.allValues) + { + entry.object = nil; + [_entryPool addObject:entry]; + } [_cache removeAllObjects]; [_lock unlock]; } +- (NSUInteger)countByEnumeratingWithState:(NSFastEnumerationState *)state + objects:(id __unsafe_unretained [])buffer + count:(NSUInteger)len +{ + [_lock lock]; + NSUInteger count = [_cache countByEnumeratingWithState:state objects:buffer count:len]; + [_lock unlock]; + return count; +} + +- (void)enumerateKeysAndObjectsUsingBlock:(void (^)(id key, id obj, BOOL *stop))block +{ + [_lock lock]; + [_cache enumerateKeysAndObjectsUsingBlock:block]; + [_lock unlock]; +} + //handle unimplemented methods -- (BOOL)isKindOfClass:(Class)cls +- (BOOL)isKindOfClass:(Class)aClass { //pretend that we're an RCTCache if anyone asks - if (cls == [RCTCache class] || cls == [NSCache class]) + if (aClass == [RCTCache class] || aClass == [NSCache class]) { return YES; } - return [super isKindOfClass:cls]; + return [super isKindOfClass:aClass]; } - (NSMethodSignature *)methodSignatureForSelector:(SEL)selector @@ -308,19 +353,16 @@ @implementation RCTCache -@dynamic count; -@dynamic totalCost; - -+ (instancetype)alloc ++ (id)alloc { return (RCTCache *)[RCTCache_Private alloc]; } -- (id)objectForKeyedSubscript:(__unused NSNumber *)key -{ - return nil; -} - +- (id)objectForKeyedSubscript:(__unused id)key { return nil; } - (void)setObject:(__unused id)obj forKeyedSubscript:(__unused id)key {} +- (void)enumerateKeysAndObjectsUsingBlock:(__unused void (^)(id, id, BOOL *))block { } +- (NSUInteger)countByEnumeratingWithState:(__unused NSFastEnumerationState *)state + objects:(__unused __unsafe_unretained id [])buffer + count:(__unused NSUInteger)len { return 0; } @end diff --git a/React/Base/RCTConvert.m b/React/Base/RCTConvert.m index 2305a1217..2ff271f64 100644 --- a/React/Base/RCTConvert.m +++ b/React/Base/RCTConvert.m @@ -396,7 +396,7 @@ RCT_CGSTRUCT_CONVERTER(CGAffineTransform, (@[ static dispatch_once_t onceToken; dispatch_once(&onceToken, ^{ colorCache = [[RCTCache alloc] init]; - colorCache.countLimit = 1024; + colorCache.countLimit = 128; }); UIColor *color = colorCache[json]; if (color) { diff --git a/React/Views/RCTView.h b/React/Views/RCTView.h index 51d060ca3..6e7019f58 100644 --- a/React/Views/RCTView.h +++ b/React/Views/RCTView.h @@ -64,7 +64,7 @@ typedef void (^RCTViewEventHandler)(RCTView *view); @property (nonatomic, assign) CGFloat borderBottomRightRadius; /** - * Border colors. + * Border colors (actually retained). */ @property (nonatomic, assign) CGColorRef borderTopColor; @property (nonatomic, assign) CGColorRef borderRightColor; diff --git a/React/Views/RCTView.m b/React/Views/RCTView.m index 1acb1b2d6..407eba24f 100644 --- a/React/Views/RCTView.m +++ b/React/Views/RCTView.m @@ -480,7 +480,7 @@ RCT_NOT_IMPLEMENTED(-initWithCoder:unused) _borderTopColor ?: _borderColor, _borderLeftColor ?: _borderColor, _borderBottomColor ?: _borderColor, - _borderRightColor ?: _borderColor + _borderRightColor ?: _borderColor, }; } @@ -580,14 +580,15 @@ RCT_NOT_IMPLEMENTED(-initWithCoder:unused) #pragma mark Border Color -#define setBorderColor(side) \ - - (void)setBorder##side##Color:(CGColorRef)border##side##Color \ - { \ - if (CGColorEqualToColor(_border##side##Color, border##side##Color)) { \ - return; \ - } \ - _border##side##Color = border##side##Color; \ - [self.layer setNeedsDisplay]; \ +#define setBorderColor(side) \ + - (void)setBorder##side##Color:(CGColorRef)color \ + { \ + if (CGColorEqualToColor(_border##side##Color, color)) { \ + return; \ + } \ + CGColorRelease(_border##side##Color); \ + _border##side##Color = CGColorRetain(color); \ + [self.layer setNeedsDisplay]; \ } setBorderColor() @@ -598,14 +599,14 @@ setBorderColor(Left) #pragma mark - Border Width -#define setBorderWidth(side) \ - - (void)setBorder##side##Width:(CGFloat)border##side##Width \ - { \ - if (_border##side##Width == border##side##Width) { \ - return; \ - } \ - _border##side##Width = border##side##Width; \ - [self.layer setNeedsDisplay]; \ +#define setBorderWidth(side) \ + - (void)setBorder##side##Width:(CGFloat)width \ + { \ + if (_border##side##Width == width) { \ + return; \ + } \ + _border##side##Width = width; \ + [self.layer setNeedsDisplay]; \ } setBorderWidth() @@ -614,14 +615,14 @@ setBorderWidth(Right) setBorderWidth(Bottom) setBorderWidth(Left) -#define setBorderRadius(side) \ - - (void)setBorder##side##Radius:(CGFloat)border##side##Radius \ - { \ - if (_border##side##Radius == border##side##Radius) { \ - return; \ - } \ - _border##side##Radius = border##side##Radius; \ - [self.layer setNeedsDisplay]; \ +#define setBorderRadius(side) \ + - (void)setBorder##side##Radius:(CGFloat)radius \ + { \ + if (_border##side##Radius == radius) { \ + return; \ + } \ + _border##side##Radius = radius; \ + [self.layer setNeedsDisplay]; \ } setBorderRadius() @@ -630,4 +631,13 @@ setBorderRadius(TopRight) setBorderRadius(BottomLeft) setBorderRadius(BottomRight) +- (void)dealloc +{ + CGColorRelease(_borderColor); + CGColorRelease(_borderTopColor); + CGColorRelease(_borderRightColor); + CGColorRelease(_borderBottomColor); + CGColorRelease(_borderLeftColor); +} + @end