billymeltdown / nsdate-helper

A category to extend Cocoa's NSDate class with some convenience functions.
http://www.zetetic.net/blog/
456 stars 107 forks source link

stringForDisplayFromDate:prefixed: returns nil in Release configuration, iOS 5.1 (possibly earlier) #7

Closed billymeltdown closed 12 years ago

billymeltdown commented 12 years ago

We've had bugs emerge in Codebook and Strip, where I use NSDate+Helper heavily, since the introduction of a static NSCalendar variable in this category's +load implementation. This can be seen with a little NSLog output on the Console in Release configuration on iOS 5.1:

Mar 28 10:59:43 unknown Codebook[11773] <Warning>: date in question: 2012-03-27 19:47:09 +0000
Mar 28 10:59:43 unknown Codebook[11773] <Warning>: today is: 2012-03-28 14:59:43 +0000
Mar 28 10:59:43 unknown Codebook[11773] <Warning>: midnight based on today and calendar components: (null)
Mar 28 10:59:43 unknown Codebook[11773] <Warning>: shared calendar object? (null)

I believe load is not being called at the same time in Release configuration for this category as it is for Debug configuration, and something about when it is being called results in the static calendar variable being null. It could be that there was no 'currentCalendar' when the category was originally loaded (likely not when my view was loaded above). While this was introduced for performance, it's not working in Release, going to have to remove the static variable for now. Open to other ideas, especially if anybody (Jim?) wants to dig into why +load isn't being called at the same time and how we could approach this differently. FWIW, performance doesn't appear to be too bad, I'd be curious to see any metrics demonstrating particularly poor performance (I hadn't noticed any issues myself before this change was introduced).

billymeltdown commented 12 years ago

Issue is resolved with commit:

commit c3a3ec6a4474e4fa3887505858ea8e0a3e5b8182 Author: Billy Gray wgray@zetetic.net Date: Wed Mar 28 12:04:23 2012 -0400

If performance continues to be an issue we should address with a shared/static variable, please open a new issue and we'll track pull requests against it.

Thanks!

billymeltdown commented 12 years ago

Some advice from Mike Ash on the topic over here:

The +load method can be dangerous like that. You really can't call out to any other Objective-C classes, because they may rely on +load too, and yours might run first.

My recommendation would be to use dispatch_once to lazily initialize your static variable. The additional overhead is insignificant and you completely avoid problems like this.

When I get a hot minute I'll give that a shot.