Skip to content

Commit

Permalink
Bunch of speed improvements for initial addressbook record accesses.
Browse files Browse the repository at this point in the history 
We now maintain a weak lookup map from  recordRef to RHRecord instances.
We also maintain a weak map from recordID to RHPerson objects.

With both these optimizations, we end up with a 28x speedup for the first call to people and 17x for subsequent people calls.

STATS:

//Original
PERF: Init took 0.445754 seconds.
PERF: First people call took 4.762057 seconds. (for 5000 people)
PERF: Second people call took 0.554423 seconds. (for 5000 people)

//With Both
PERF: Init took 0.462656 seconds.
PERF: First people call took 0.166239 seconds. (for 5000 people)
PERF: Second people call took 0.030911 seconds. (for 5000 people)

//With ID Map Only
PERF: Init took 0.453450 seconds.
PERF: First people call took 1.004441 seconds. (for 5000 people)
PERF: Second people call took 0.919334 seconds. (for 5000 people)

//With Ref Map Only
PERF: Init took 0.499064 seconds.
PERF: First people call took 5.689294 seconds. (for 5000 people)
PERF: Second people call took 0.050356 seconds. (for 5000 people)
  • Loading branch information
@heardrwt
heardrwt committed Nov 12, 2012
1 parent 7ab3c2d commit 642a851
Show file tree
Hide file tree
Showing 3 changed files with 216 additions and 26 deletions.
201 changes: 175 additions & 26 deletions RHAddressBook/RHAddressBook.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@
#import "RHAddressBookThreadMain.h"
#import "RHAddressBook_private.h"

#define USE_REF_MAP 1
#define USE_PERSON_ID_MAP 1

NSString * const RHAddressBookExternalChangeNotification = @"RHAddressBookExternalChangeNotification";
NSString * const RHAddressBookPersonAddressGeocodeCompleted = @"RHAddressBookPersonAddressGeocodeCompleted";

Expand All @@ -54,6 +57,10 @@ -(NSArray*)peopleForABRecordRefs:(CFArrayRef)peopleRefs; //bulk performer

-(void)addressBookExternallyChanged:(NSNotification*)notification; //notification on external changes. (revert if no local changes so always up-to-date)

#if USE_PERSON_ID_MAP
-(void)rebuildPersonIDToRecordMap:(BOOL)waitTillDone;
#endif

@end

@implementation RHAddressBook {
Expand Down Expand Up @@ -89,7 +96,20 @@ @implementation RHAddressBook {
addressbook for functionality that would break if the addressbook went away)
*/

#if USE_REF_MAP
//ref to record weak map. (we maintain this for faster access directly to RHRecord objects)
CFMutableDictionaryRef _refToRecordMap;

#endif

#if USE_PERSON_ID_MAP
//further optimizations specifically for looking up RHPerson records based on recordID
CFMutableDictionaryRef _personIDToRecordMap;

#endif


}

@synthesize addressBookThread=_addressBookThread;
Expand Down Expand Up @@ -142,6 +162,14 @@ -(id)init{
_sources = (__bridge_transfer NSMutableSet *)CFSetCreateMutable(NULL, 0, NULL);
_groups = (__bridge_transfer NSMutableSet *)CFSetCreateMutable(NULL, 0, NULL);
_people = (__bridge_transfer NSMutableSet *)CFSetCreateMutable(NULL, 0, NULL);

#if USE_REF_MAP
_refToRecordMap = CFDictionaryCreateMutable(nil, 0, NULL, NULL); //weak for both keys and values
#endif

#if USE_PERSON_ID_MAP
_personIDToRecordMap = CFDictionaryCreateMutable(nil, 0, &kCFTypeDictionaryKeyCallBacks, NULL); //weak for both keys and values
#endif
}];

//subscribe to external change notifications
Expand All @@ -166,6 +194,15 @@ -(void)dealloc{
arc_release_nil(_sources);
arc_release_nil(_groups);
arc_release_nil(_people);

#if USE_REF_MAP
if (_refToRecordMap) CFRelease(_refToRecordMap); _refToRecordMap = NULL;
#endif

#if USE_PERSON_ID_MAP
if (_personIDToRecordMap) CFRelease(_personIDToRecordMap); _personIDToRecordMap = NULL;
#endif

arc_super_dealloc();
}

Expand Down Expand Up @@ -263,6 +300,21 @@ -(RHSource*)sourceForABRecordRef:(ABRecordRef)sourceRef{
// these not yet saved objects are added to the cache via the weak record check in / out system when they are created / dealloc'd


#if USE_REF_MAP

//look for an exact match using recordRef
__block RHSource *source = nil;
[_addressBookThread rh_performBlock:^{
id mapSource = CFDictionaryGetValue(_refToRecordMap, sourceRef);
if ([mapSource isKindOfClass:[RHSource class]]){
source = arc_retain(mapSource);
}
}];

if (source) return arc_autorelease(source);

#else

//search for an exact match using recordRef
__block RHSource *source = nil;
[_addressBookThread rh_performBlock:^{
Expand All @@ -278,7 +330,7 @@ -(RHSource*)sourceForABRecordRef:(ABRecordRef)sourceRef{

if (source) return arc_autorelease(source);


#endif

//get the sourceID
__block ABRecordID sourceID = kABRecordInvalidID;
Expand Down Expand Up @@ -388,6 +440,21 @@ -(RHGroup*)groupForABRecordRef:(ABRecordRef)groupRef{
// these not yet saved objects are added to the cache via the weak record check in / out system when they are created / dealloc'd


#if USE_REF_MAP

//look for an exact match using recordRef
__block RHGroup *group = nil;
[_addressBookThread rh_performBlock:^{
id mapGroup = CFDictionaryGetValue(_refToRecordMap, groupRef);
if ([mapGroup isKindOfClass:[RHGroup class]]){
group = arc_retain(mapGroup);
}
}];

if (group) return arc_autorelease(group);

#else

//search for an exact match using recordRef
__block RHGroup *group = nil;
[_addressBookThread rh_performBlock:^{
Expand All @@ -402,7 +469,8 @@ -(RHGroup*)groupForABRecordRef:(ABRecordRef)groupRef{
}];

if (group) return arc_autorelease(group);


#endif

//if no direct match found, try using recordID
__block ABRecordID groupID = kABRecordInvalidID;
Expand Down Expand Up @@ -511,7 +579,7 @@ -(NSArray*)peopleOrderedBySortOrdering:(ABPersonSortOrdering)ordering{

}

CFRelease(peopleRefs);
CFRelease(peopleRefs);

}
}];
Expand Down Expand Up @@ -550,6 +618,21 @@ -(RHPerson*)personForABRecordRef:(ABRecordRef)personRef{
// these not yet saved objects are added to the cache via the weak record check in / out system when they are created / dealloc'd


#if USE_REF_MAP

//look for an exact match using recordRef
__block RHPerson *person = nil;
[_addressBookThread rh_performBlock:^{
id mapPerson = CFDictionaryGetValue(_refToRecordMap, personRef);
if ([mapPerson isKindOfClass:[RHPerson class]]){
person = arc_retain(mapPerson);
}
}];

if (person) return arc_autorelease(person);

#else

//search for an exact match using recordRef
__block RHPerson *person = nil;
[_addressBookThread rh_performBlock:^{
Expand All @@ -565,6 +648,7 @@ -(RHPerson*)personForABRecordRef:(ABRecordRef)personRef{

if (person) return arc_autorelease(person);

#endif

//if exact matching failed, look using recordID;
__block ABRecordID personID = kABRecordInvalidID;
Expand All @@ -578,7 +662,13 @@ -(RHPerson*)personForABRecordRef:(ABRecordRef)personRef{

//search for the actual person using recordID
[_addressBookThread rh_performBlock:^{

#if USE_PERSON_ID_MAP

id mapPerson = CFDictionaryGetValue(_personIDToRecordMap, (__bridge const void *)([NSNumber numberWithInt:personID]));
if (mapPerson) person = arc_retain(mapPerson);

#else
//look in the cache
for (RHPerson *entry in _people) {
//compare using ID not ref
Expand All @@ -588,6 +678,8 @@ -(RHPerson*)personForABRecordRef:(ABRecordRef)personRef{
}
}

#endif

//if not in the cache, create and add a new one
if (! person){

Expand Down Expand Up @@ -729,7 +821,7 @@ -(NSArray*)addPeopleFromVCardRepresentation:(NSData*)representation toSource:(RH
}
if (peopleRefs) CFRelease(peopleRefs);
}];
return newPeople;
return newPeople;
}

-(NSData*)vCardRepresentationForPeople:(NSArray*)people{
Expand Down Expand Up @@ -822,6 +914,10 @@ -(BOOL)save:(NSError**)error{
if (cfError) CFRelease(cfError);
}

#if USE_PERSON_ID_MAP
[self rebuildPersonIDToRecordMap:YES];
#endif

return result;
}

Expand All @@ -841,7 +937,7 @@ -(void)revert{
}

-(void)addressBookExternallyChanged:(NSNotification*)notification{
//notification on external changes. (revert if no local changes so always up-to-date)
//notification on external changes. (revert if no local changes so always up-to-date)
if (![self hasUnsavedChanges]){
[self revert];
} else {
Expand All @@ -850,6 +946,27 @@ -(void)addressBookExternallyChanged:(NSNotification*)notification{

}


#if USE_PERSON_ID_MAP

-(void)rebuildPersonIDToRecordMap:(BOOL)waitTillDone{
[_addressBookThread rh_performBlock:^{

CFDictionaryRemoveAllValues(_personIDToRecordMap);

for (RHPerson *person in _people) {
if (person.recordID != kABRecordInvalidID){
//add the person record to the id map
CFDictionarySetValue(_personIDToRecordMap, (__bridge const void *)([NSNumber numberWithInt:person.recordID]), (__bridge const void *)(person));
}
}

} waitUntilDone:waitTillDone];

}

#endif

#pragma mark - prefs
+(ABPersonSortOrdering)sortOrdering{
return ABPersonGetSortOrdering();
Expand Down Expand Up @@ -952,20 +1069,36 @@ -(void)_recordCheckIn:(RHRecord*)record{

[_addressBookThread rh_performBlock:^{

//if source, add to _sources
if ([record isKindOfClass:[RHSource class]]){
if (![_sources containsObject:record]) [_sources addObject:record];
}
#if USE_REF_MAP
//add it to the record Ref map
CFDictionarySetValue(_refToRecordMap, record.recordRef, (__bridge const void *)(record));
#endif

//if group, add to _groups
if ([record isKindOfClass:[RHGroup class]]){
if (![_groups containsObject:record]) [_groups addObject:record];
#if USE_PERSON_ID_MAP
if ([record isKindOfClass:[RHPerson class]] && record.recordID != kABRecordInvalidID){
//add the person record to the id map
CFDictionarySetValue(_personIDToRecordMap, (__bridge const void *)([NSNumber numberWithInt:record.recordID]), (__bridge const void *)(record));
}

#endif

//if person, add to _people
if ([record isKindOfClass:[RHPerson class]]){
if (![_people containsObject:record]) [_people addObject:record];
}
[_people addObject:record];
return;
}

//if group, add to _groups
if ([record isKindOfClass:[RHGroup class]]){
[_groups addObject:record];
return;
}

//if source, add to _sources
if ([record isKindOfClass:[RHSource class]]){
[_sources addObject:record];
return;
}

}];

arc_release(record);
Expand All @@ -979,20 +1112,36 @@ -(void)_recordCheckOut:(RHRecord*)record{

[_addressBookThread rh_performBlock:^{

//if source, remove from _sources
if ([_safeRecord isKindOfClass:[RHSource class]]){
if ([_sources containsObject:_safeRecord]) [_sources removeObject:_safeRecord];
}

//if group, remove from _groups
if ([_safeRecord isKindOfClass:[RHGroup class]]){
if ([_groups containsObject:_safeRecord]) [_groups removeObject:_safeRecord];
#if USE_REF_MAP
//remove it from the map
CFDictionaryRemoveValue(_refToRecordMap, _safeRecord.recordRef);
#endif

#if USE_PERSON_ID_MAP
if ([_safeRecord isKindOfClass:[RHPerson class]]){
//remove it from the id map
CFDictionaryRemoveValue(_personIDToRecordMap, (__bridge const void *)([NSNumber numberWithInt:_safeRecord.recordID]));
}

#endif

//if person, remove from _people
if ([_safeRecord isKindOfClass:[RHPerson class]]){
if ([_people containsObject:_safeRecord]) [_people removeObject:_safeRecord];
}
[_people removeObject:_safeRecord];
return;
}

//if group, remove from _groups
if ([_safeRecord isKindOfClass:[RHGroup class]]){
[_groups removeObject:_safeRecord];
return;
}

//if source, remove from _sources
if ([_safeRecord isKindOfClass:[RHSource class]]){
[_sources removeObject:_safeRecord];
return;
}

}];

}
Expand Down
35 changes: 35 additions & 0 deletions RHAddressBookLogicTests/RHAddressBookLogicTests.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1446,6 +1446,41 @@ -(void)testWeakLinkedCache{
[_ab revert];
}


-(void)testWeakLinkedRefMap{

//setup (get ivar refs)
CFMutableDictionaryRef _refToRecordMap = (CFMutableDictionaryRef)[self ivar:@"_refToRecordMap" forObject:_ab];

if (_refToRecordMap){

//add a group, make sure its added to map
RHGroup *newGroup = nil;
RHPerson *newPerson = nil;
@autoreleasepool {
newGroup = [_ab newGroupInDefaultSource];
newGroup.name = @"Unit Test GroupC";
newPerson = [_ab newPersonInDefaultSource];
}
STAssertTrue(CFDictionaryGetValue(_refToRecordMap, newGroup.recordRef) != NULL, @"_refToRecordMap does not contain weak ref to newGroup");
STAssertTrue(CFDictionaryGetValue(_refToRecordMap, newPerson.recordRef) != NULL, @"_refToRecordMap does not contain weak ref to newPerson");

//release group make sure its removed from map
ABRecordRef newGroupRef = newGroup.recordRef;
ABRecordRef newPersonRef = newPerson.recordRef;

[newGroup release];
[newPerson release];

[[NSRunLoop currentRunLoop] runUntilDate:[NSDate date]];
STAssertFalse(CFDictionaryGetValue(_refToRecordMap, newGroupRef) != NULL, @"_refToRecordMap still contains weak ref to newGroup");
STAssertFalse(CFDictionaryGetValue(_refToRecordMap, newPersonRef) != NULL, @"_refToRecordMap still contains weak ref to newPerson");
}
//cleanup
[_ab revert];
}


//we only want these tests to run if linked against 5.0+ hence the defines and also, only if we are running on less that 5.0
#if __IPHONE_OS_VERSION_MAX_ALLOWED >= 50000
#pragma mark - running on pre iOS5+ sanity
Expand Down
Loading

0 comments on commit 642a851

Please sign in to comment.