From 864a19b8a1be2d0162de0b8df8cb1e9509425090 Mon Sep 17 00:00:00 2001 From: Karl Williamson Date: Wed, 10 Jan 2024 11:30:11 -0700 Subject: [PATCH] Don't use both mini_mktime() and mkttime() mini_mktime() has less overhead than libc mkttime(), but it doesn't get all the desired fields on all platforms. Prior to this commit, both got called on such platforms, roughly doubling the amount of time spent. So just use the slightly more expensive function on those platforms and avoid that redundant overhead. The only glitch I've seen is that mktime() likely doesn't handle leap seconds the same way mini_mktime() does; so this calls both functions for those very rare instances. --- locale.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 61 insertions(+), 8 deletions(-) diff --git a/locale.c b/locale.c index dfd0d2280277..6e1172d8e688 100644 --- a/locale.c +++ b/locale.c @@ -7877,22 +7877,75 @@ S_ints_to_tm(pTHX_ struct tm * mytm, mytm->tm_wday = wday; mytm->tm_yday = yday; mytm->tm_isdst = isdst; + +/* If no mktime() can't call it; and if it doesn't have the extra fields that + * mini_mktime() doesn't handle, no need to call it; use our faster internal + * mini_mktime() instead */ +#if ! defined(HAS_MKTIME) || ( ! defined(HAS_TM_TM_GMTOFF) \ + && ! defined(HAS_TM_TM_ZONE)) + mini_mktime(mytm); - /* use libc to get the values for tm_gmtoff and tm_zone on platforms that - * have them [perl #18238] */ -#if defined(HAS_MKTIME) \ - && (defined(HAS_TM_TM_GMTOFF) || defined(HAS_TM_TM_ZONE)) - struct tm mytm2 = *mytm; +#else + + /* Otherwise we have to use the (slower) libc mktime(), which does take + * locale into consideration. [perl #18238] */ MKTIME_LOCK; - mktime(&mytm2); + + (void) mktime(mytm); + MKTIME_UNLOCK; + + /* More is needed if this is the very unlikely case of a leap second + * (sec==60). Various mktime() routines handle these differently: + * a) Some don't know about their existence and normalize them to 0 + * seconds in the next minute; + * b) Others do the same, except if a table lookup indicates there + * actually was an officially declared leap second at the instant the + * input gives, in which case they leave the second at 60, and don't + * adjust the minute. + * c) mini_mktime takes the caller's word for it that this is an actual + * leap second, and leaves it at 60. + * In all cases, if the input was 60 and the result is not, libc did not + * act like mini_mktime, and to preserve long-standing behavior, we call + * mini_mktime for its result. + * + * (As an aside, this means the return of these, if we didn't ignore it + * here, is not consistent across platforms.) */ + if (UNLIKELY(sec == 60 && mytm->tm_sec != 60)) { + + /* But since mini_mktime() doesn't handle these fields, save the + * results we already have for them */ +# ifdef HAS_TM_TM_GMTOFF + long int gmtoff = mytm->tm_gmtoff; +# endif +# ifdef HAS_TM_TM_ZONE + const char *zone = mytm->tm_zone; +# endif + + /* Repeat the entire process with mini_mktime */ + Zero(mytm, 1, struct tm); + mytm->tm_sec = sec; + mytm->tm_min = min; + mytm->tm_hour = hour; + mytm->tm_mday = mday; + mytm->tm_mon = mon; + mytm->tm_year = year; + mytm->tm_wday = wday; + mytm->tm_yday = yday; + mytm->tm_isdst = isdst; + (void) mini_mktime(mytm); + + /* And use the saved libc values for tm_gmtoff and tm_zone */ # ifdef HAS_TM_TM_GMTOFF - mytm->tm_gmtoff = mytm2.tm_gmtoff; + mytm->tm_gmtoff = gmtoff; # endif # ifdef HAS_TM_TM_ZONE - mytm->tm_zone = mytm2.tm_zone; + mytm->tm_zone = zone; # endif + + } + #endif return;