Re: [PATCH] Remove unncessary localtime() calls during data type conversion

On Sat, Jun 13, 2015 at 9:42 AM, Nikhil Deshpande <ndeshpande@vmware.com> wrote:
> While doing some profiling work on an ODBC client, we noticed tons
> of libc localtime() and __tz_convert() calls on a hot code path
> (SQLFetch()), but most of the fields being retrieved were not of
> date/time type.
>
>
> These libc calls are costly (non-trivial work in terms of string ops,
> timzone conversion etc.) and the cost adds up quickly for high-frequency
> call like SQLFetch(). We noticed that this penalty is paid even
> for data types which aren't date/time related. Code inspection
> showed localtime() call in convert.c:copy_and_convert_field()
> at function start, being invoked regardless of whether output of
> localtime() is used or not.
>
> Attached is a non-intrusive patch that moves the call down
> to where it's needed (for both getting data from server and
> sending data to server), but I'm not sure if the patch is complete
> (it covers the case where source data type is time but
> destination needs date part, "TIME->DATE[TIME]").
>
> Could you please review this patch and modify it for correctness
> and consider for committing it?

Indeed. I would move as well the declarations of tim more within the
inner loops.

> A longer term cleaner approach would be to refactor the code to aim for
> at most one call to localtime() (e.g. if it's already invoked once
> in copy_and_convert_field() before, reuse that data). However
> that is much more intrusive change.

Yep. This would be saner long-term.
--
Michael