Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store NSDateFormatters in thread-attached storage to avoid wasteful + ex... #296

Merged
merged 3 commits into from
Jun 24, 2014

Conversation

rspeyer
Copy link
Contributor

@rspeyer rspeyer commented Jun 23, 2014

...pensive recreation of these objects.

In my experience, it can be very costly to instantiate NSDateFormatter objects and there is no need since they are almost-always (like here) created using constant input parameters. We use thread-attached storage here because the formatter objects are documented as being non thread-safe, thus the best we can do is once instance per thread.

@bpoplauschi
Copy link
Member

@rspeyer indeed, creating NSDateFormatter or other NSFormatter subclass is expensive and those objects are not thread safe. We are using the formatter just for log file names (that is not very often), but from a performance POV, that would be the correct way to write that reusable formatter. I just have a comment. Could you please compose logFileDateFormatter and the log format (in this case yyyy'-'MM'-'dd' 'HH'-'mm') for the thread dictionary key? This way, if we decide to add another formatter with a different format, it won't collide with this one.

@bpoplauschi bpoplauschi added this to the 1.9.1 milestone Jun 23, 2014
@rspeyer
Copy link
Contributor Author

rspeyer commented Jun 23, 2014

Thanks @bpoplauschi. Fixed.

@bpoplauschi
Copy link
Member

I meant

NSMutableDictionary *dictionary = [[NSThread currentThread] threadDictionary];
NSString *dateFormat = @"yyyy'-'MM'-'dd' 'HH'-'mm'";
NSString *threadDictKey = [@"logFileDateFormatter." stringByAppendingString:dateFormat];
NSDateFormatter *dateFormatter = [dictionary objectForKey:threadDictKey];
if (dateFormatter == nil) {
    NSDateFormatter *dateFormatter = [[NSDateFormatter alloc] init];
    [dateFormatter setDateFormat:dateFormat];
    [dateFormatter setTimeZone:[NSTimeZone timeZoneForSecondsFromGMT:0]];
    [dictionary setObject:dateFormatter forKey:key];
}

no need to copy paste the dateFormat.

@rspeyer
Copy link
Contributor Author

rspeyer commented Jun 23, 2014

Done

@bpoplauschi
Copy link
Member

Great. Merging.

bpoplauschi added a commit that referenced this pull request Jun 24, 2014
Store NSDateFormatters in thread-attached storage to avoid wasteful + ex...
@bpoplauschi bpoplauschi merged commit bec5f56 into CocoaLumberjack:master Jun 24, 2014
@rspeyer rspeyer deleted the formatter branch June 24, 2014 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants