Re: [Tracker] [PATCH] Improve performance of log_output()

[Jim Kukunas <james t kukunas linux intel com>]

On Tue, Mar 29, 2011 at 12:38:16PM +0200, Jurg Billeter wrote:
> Hi Jim,
>
> On Fri, 2011-03-25 at 18:18 -0700, Jim Kukunas wrote:
> > While profiling CPU usage, I noticed that everytime tracker writes
> > to a log file, it invokes localtime(3) to generate the timestamp. This
> > is less than ideal, because localtime(3) is a relatively expensive
> > function.
>
> The idea is that tracker is running with low log verbosity during
> regular operation, at least on embedded devices. Due to that, I don't
> consider log performance very important myself. I also expect the I/O
> part of logging to be more expensive than localtime(3). However, maybe
> there is a reason why you need to enable verbose logging in a situation
> where performance matters, so let's discuss the patch.

I absolutely agree with you. My goal is to improve overall tracker
performance. This was at the top of one of my perf profiles, so I figured
it was a good place to start.

> > The attached patch changes the behavior in tracker-log.c to invoke
> > localtime(3) only once, during the invocation of tracker_log_init, and
> > then use gettimeofday to update the time relative to the time returned
> > by the initial localtime(3).
>
> If I read the patches correctly, it will print incorrect timestamps if
> the UTC offset changes (DST or timezone change) after tracker_log_init.
> While this might not happen very often, I consider it important that the
> output is correct as debugging could be very confusing with wrong
> timestamps. I assume that timezone handling is also the reason for
> localtime(3) being slow.

Ah, you are correct. These patches do not properly handle that case.

> In the case that log performance is important to you, it might make more
> sense to optionally support logging with Unix or UTC timestamps. The
> former would definitely be faster, the latter presumably as well.

That sounds like a much more elegant solution.

> Regards,
> Jurg

Thanks for reviewing the patches, and for the feedback.

-- 
Jim Kukunas
Intel Open Source Technology Center