System.Tests.Perf_DateTime.GetNow gets slower every summer due to daylight savings #67932
Comments
|
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
dotnet-issue-labeler
bot
added
the
untriaged
|
This is expected, |
|
this code only runs when we're in DST: https://github.com/dotnet/runtime/blob/67761043f1b0e1514a1b8c1c6d99a353737a24a1/src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.cs#L1383-L1412 But, that doesn't seem like much. would need profiling. |
|
|
|
oh, or maybe it does? |
|
Is that with DST, or without? If GetDatePart is larger in DST, then it's perhaps because it's getting Year out of the DateTime in that block. The code before already gets Day and Month and Year, though. I wonder if there is opportunity to have GetDatePart() return all of those together rather than repeat its work. (Edit - I see that's GetDate()) is the codegen for GetDatePart() reasonable? |
|
Ah, ignore my flamegraph - it's for my local time where we don't have daylight savings. When I change my tz to the pacific one I now see a different picture - it seems there are quite a few things to optimize
more or less, but we might need to extract it for Year separately since it's on a hot path and inline into |
|
Tagging subscribers to this area: @dotnet/area-system-runtime Issue DetailsSystem.Tests.Perf_DateTime.GetNow has regressed across multiple configurations.
Looking at the commit diff, #66282 seems to be to blame. Repro: git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_monthly.py net7.0-preview2 net7.0-preview3 --filter System.Tests.Perf_DateTime.GetNow
## System.Tests.Perf_DateTime.GetNow
|
|
I'll take a look when I have some spare time 🙂 |
ghost
removed
the
danmoseley
added
the
help wanted
danmoseley
changed the title
|
I measured the impact locally, including old frameworks for interest. Difference is only about 4% now, in .NET 6 it was 9% which is what the graph above shows from earlier. The recent #72712 will have contributed to that, because the DST path adds calls to Year, Month, Day. @tarekgh it is OK to assume that timezone rules aren't changing during the lifetime of the process, right? So possibly the current rule and timezone can be cached, and if the timezone matches what was cached, we can continue to use that rule, and maybe short circuit simple cases. I'll leave as help wanted in case someone wants to take a look at some kind of caching of that sort. But otherwise I am not sure it is worth further attention. benchmarkusing System;
using System.ComponentModel;
using System.Runtime.InteropServices;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Configs;
using BenchmarkDotNet.Environments;
using BenchmarkDotNet.Jobs;
using BenchmarkDotNet.Running;
[MemoryDiagnoser]
public class Program
{
// run as admin
public static void Main(string[] args)
{
var config = DefaultConfig.Instance
.AddJob(Job.Default.WithRuntime(CoreRuntime.Core31))
.AddJob(Job.Default.WithRuntime(CoreRuntime.Core60))
.AddJob(Job.Default.WithRuntime(CoreRuntime.CreateForNewVersion("net7.0", ".NET 7.0")))
.AddJob(Job.Default.WithRuntime(ClrRuntime.Net48));
BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args, config);
}
[GlobalSetup(Target = nameof(DateTimeNow_NoDST))]
public void NoDST() => SetSystemTime(new DateTime(2022, 12, 1)); // assuming US
[GlobalSetup(Target = nameof(DateTimeNow_DST))]
public void DST() => SetSystemTime(new DateTime(2022, 6, 1)); // assuming US
[Benchmark(Baseline = true)]
public DateTime DateTimeNow_NoDST() => DateTime.Now;
[Benchmark]
public DateTime DateTimeNow_DST() => DateTime.Now;
public void SetSystemTime(DateTime dt)
{
SYSTEMTIME st = new SYSTEMTIME();
st.wYear = (short)dt.Year;
st.wMonth = (short)dt.Month;
st.wDay = (short)dt.Day;
SetSystemTime(ref st);
int errorCode = Marshal.GetLastWin32Error();
if (errorCode != 0)
throw new Win32Exception(errorCode);
}
[DllImport("kernel32.dll", SetLastError = true)]
private static extern bool SetSystemTime(ref SYSTEMTIME st);
}
[StructLayout(LayoutKind.Sequential)]
public struct SYSTEMTIME
{
public short wYear;
public short wMonth;
public short wDayOfWeek;
public short wDay;
public short wHour;
public short wMinute;
public short wSecond;
public short wMilliseconds;
}Here's an idea of where the time is spent. There's not a clear "single place". |
Not really, users can change the local time zone and clear the cached Local TZ. But we can handle flushing the current rule if the local TZ cache is flushed. By the way, on Windows, we already cache the rule The better idea here is not catching the rule and only cache the UTC offset and when the daylight will change. Doing that will make calculation of the |
danmoseley
removed
the
help wanted
danmoseley
added
the
help wanted
System.Tests.Perf_DateTime.GetNow has regressed across multiple configurations.
Reporting System:
Looking at the commit diff, #66282 seems to be to blame.
Repro:
The text was updated successfully, but these errors were encountered: