[PATCH 0/1] MR10348: ntdll: Avoid expensive inverted DST check on get_timezone_info() cache hit path

[Zhou Qiankang (＠wszqkzqk)]

inverted DST detection (two mktime() calls) out of the cache hit fast path in
get_timezone_info().

Keep the cache key based on the cheap raw bias value and only compute
inverted_dst/adjusted_bias on cache miss. Preserve the inverse DST correctness
fix by still applying adjusted_bias to tzi->Bias in the slow path.

This restores fast-path behavior for frequent GetTimeZoneInformation callers without regressing inverse DST handling.

Fixes regression: bfae6eb8b4f6 ("ntdll: Fix get_timezone_info() for timezones defined with inverse DST on Unix.")

Fixes: https://bugs.winehq.org/show_bug.cgi?id=59514

Signed-off-by: Zhou Qiankang <[e-mail]>

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10348

[PATCH 1/1] ntdll: Avoid expensive inverted DST check on get_timezone_info() cache hit path

[Zhou Qiankang]

From: Zhou Qiankang <[e-mail]>

inverted DST detection (two mktime() calls) out of the cache hit fast path in
get_timezone_info().

Keep the cache key based on the cheap raw bias value and only compute
inverted_dst/adjusted_bias on cache miss. Preserve the inverse DST correctness
fix by still applying adjusted_bias to tzi->Bias in the slow path.

This restores fast-path behavior for frequent GetTimeZoneInformation callers without regressing inverse DST handling.

Fixes regression: bfae6eb8b4f6 ("ntdll: Fix get_timezone_info() for timezones defined with inverse DST on Unix.")

Fixes: https://bugs.winehq.org/show_bug.cgi?id=59514

Signed-off-by: Zhou Qiankang <[e-mail]>
---
 dlls/ntdll/unix/system.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/dlls/ntdll/unix/system.c b/dlls/ntdll/unix/system.c
index 22f8b496220..f94720ce02a 100644
--- a/dlls/ntdll/unix/system.c
+++ b/dlls/ntdll/unix/system.c
@@ -3002,7 +3002,7 @@ static void get_timezone_info( RTL_DYNAMIC_TIME_ZONE_INFORMATION *tzi )
     RTL_DYNAMIC_TIME_ZONE_INFORMATION reg_tzi;
     struct tm *tm, tm1, tm2;
     time_t year_start, year_end, tmp, dlt = 0, std = 0;
-    int is_dst, bias;
+    int is_dst, bias, adjusted_bias;
     BOOL inverted_dst;
 
     mutex_lock( &timezone_mutex );
@@ -3012,12 +3012,6 @@ static void get_timezone_info( RTL_DYNAMIC_TIME_ZONE_INFORMATION *tzi )
     bias = (LONG)(mktime(tm) - year_start) / 60;
 
     tm = localtime(&year_start);
-    tm1 = tm2 = *tm;
-    tm1.tm_isdst = 0;
-    tm2.tm_isdst = 1;
-    inverted_dst = mktime(&tm1) < mktime(&tm2);
-    if (inverted_dst) bias += 60;
-
     if (current_year == tm->tm_year && current_bias == bias)
     {
         *tzi = cached_tzi;
@@ -3025,12 +3019,20 @@ static void get_timezone_info( RTL_DYNAMIC_TIME_ZONE_INFORMATION *tzi )
         return;
     }
 
+    tm1 = tm2 = *tm;
+    tm1.tm_isdst = 0;
+    tm2.tm_isdst = 1;
+    inverted_dst = mktime(&tm1) < mktime(&tm2);
+    adjusted_bias = bias;
+    if (inverted_dst) adjusted_bias += 60;
+
     memset(tzi, 0, sizeof(*tzi));
-    TRACE("tz data will be valid through year %d, bias %d, inverted_dst %d\n", tm->tm_year + 1900, bias, inverted_dst);
+    TRACE("tz data will be valid through year %d, bias %d, adjusted_bias %d, inverted_dst %d\n",
+          tm->tm_year + 1900, bias, adjusted_bias, inverted_dst);
     current_year = tm->tm_year;
     current_bias = bias;
 
-    tzi->Bias = bias;
+    tzi->Bias = adjusted_bias;
 
     tm->tm_isdst = inverted_dst;
     tm->tm_mday = 1;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/10348

[Paul Gofman (＠gofman)]

Paul Gofman (@gofman) commented about dlls/ntdll/unix/system.c:
> RTL_DYNAMIC_TIME_ZONE_INFORMATION reg_tzi;
>      struct tm *tm, tm1, tm2;
>      time_t year_start, year_end, tmp, dlt = 0, std = 0;
> -    int is_dst, bias;
> +    int is_dst, bias, adjusted_bias;

You don't need an extra variable, it is possible to just assign the bias to static current_bias before adjusting it for inverted DST.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10348#note_132373

[Paul Gofman (＠gofman)]

Thanks for spotting this and the fix! I checked that basically the same is observed here with the tast, it is mktime() with switched tm_isdst fields takes abysmal time in glibc. Perhaps not less important, the same inverted DST check is also now present in wineserver (set_user_shared_data_time), I sent a patch to avoid computing that each second in server too: https://gitlab.winehq.org/wine/wine/-/merge_requests/10350

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10348#note_132375