RestKit/RestKit — pull request #2068 by carllindberg (2014-10-29 07:45:15+00:00)
Title: Speed up RKDictionaryByMergingDictionaryWithDictionary a bit.  Primarily...

Speed up RKDictionaryByMergingDictionaryWithDictionary a bit. After the RKCompoundValueTransformer fix, this function is the biggest hotspot in the profiling results. It's not huge, but it's at the top.

The major change is to avoid the copy at the end; since this is a local, transient value, it doesn't seem necessary at all. Since most calls to the function include a nested dictionary, that would be at least two saved dictionary copies per call. RKMappingOperation calls it twice every time a new sub-RKMappingOperation is created, and calls it in two other places as well during nested operations.

I changed the property on RKObjectMapping to be strong, rather than copy, again to avoid a copy being made (since the return value is now mutable). These are temporary objects completely owned by the mapping code, so I'm not sure how external code could modify them accidentally.

Also, I used the objectForKey: and setObject:forKey: rather than the valueForKey: and setValue:forKey: methods, as those should be more direct, and the value being set can never be nil (the usual reason to use setValue:forKey:). Also, if there so happened to be a key of the string "@count", valueForKey: will return the wrong thing.

Lastly, I changed to use fast enumeration instead of the enumerateWithBlock method. The block methods on NSArray and NSDictionary, while great, also create an autorelease pool in their loop, which does cost a little bit of time to set up. In this case, since it requires an extra call to objectForKey:, it ends up being close to a wash. I think the fast enumeration was still just slightly faster, but I probably did not make enough runs to really tell for sure. It makes a larger difference for NSArray calls, and that may be a future pull request, as there are a number of them in RestKit.

Using CFDictionaryApplyFunction, which iterates without an autorelease pool, was a bit faster still (39.762720 seconds with the PR code, and 39.489260 with the CFDictionary function). But, that is much uglier-looking code, which requires most of the logic to be in a helper function (which itself would recursively call RKDictionaryByMergingDictionaryWithDictionary), so I wasn't sure that was the best approach. It's a bit of a shame we couldn't just use addEntriesFromDictionary, which would be the fastest, but that would not do recursive merges.

This is another PR which came out of the performance issues noted on https://github.com/RestKit/RestKit/issues/2065 . When using the test application posted at https://github.com/rtimpone/restkit_relationship_mapping_benchmarking , I get the following timings on my device (an iPad 3) and iOS simulator (timings in seconds, and after previous PR fixes were added):

(Device) Mapping 5000 students with relationship mapping: 41.293718
(Simulator) Mapping 5000 students with relationship mapping: 5.952791

After applying this fix, the results are now more like:

(Device) Mapping 5000 students with relationship mapping: 39.762720
(Simulator) Mapping 5000 students with relationship mapping: 5.673784

Even after this though, RKDictionaryByMergingDictionaryWithDictionary is still the biggest hotspot, just not as much. It may be worth finding an alternate way to implement the @metadata feature rather than continually merging dictionaries. Maybe RKSourceObject or RKMappingOperation could store some of these values as separate properties, and it could have some special valueForKey: magic to obtain the values if used, instead of the dictionary approach. I don't understand the feature enough to know if that is really feasible or where the best place would be. However, it probably bears further investigation because it is still a performance bottleneck a bit.