dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.36k stars 4.75k forks source link

DateTime.Now takes 1 second to execute on WSL #24839

Open jkotas opened 6 years ago

jkotas commented 6 years ago

Repro (windows subsystem for Linux):

Result: The app takes 1.5s to run. 1 second out of that is time spent inside DateTime.Now

jkotas commented 6 years ago

The problem is that the TimeZoneInfo implementation ends up reading all timezones on the system. There is a lot of them and when it is combined with slower I/O of WSL, it takes 1 second to do it all.

We should look into making this more efficient - e.g. read all timezones only when it is actually required; or not read them at all.

jkotas commented 6 years ago

This affects other Linux distributions as well, not just WSL.

tarekgh commented 6 years ago

CC @eerhardt

eerhardt commented 6 years ago

@jkotas - I don't have WSL set up on my machine, but can you answer a few questions?

Normally (or at least on my Ubuntu machine), /etc/localtime is a symlink to a zoneinfo file. Can you readlink /etc/localtime and tell me the results?

I am assuming the reason it is slow is because TryGetLocalTzFile is trying to get the timezone ID, and it is failing to get the ID using readlink, so it falls back to scanning the /usr/share/zoneinfo folder for a matching timezone file.

Another possibility is that the TZ environment variable is set to an absolute path, and the ID can't be discerned from that path.

jkotas commented 6 years ago

Can you readlink /etc/localtime and tell me the results?

Right - it is not a symlink. @wfurt could you please comment on what other distros have this as a file, and not symlink?

it falls back to scanning the /usr/share/zoneinfo folder for a matching timezone file.

Right. The code needs to figure out the timezone id just to make some internal implementation detail plumbing happy. Can we delay this until after somebody actually asks about the timezone id?

jkotas commented 6 years ago

I don't have WSL set up on my machine, but can you answer a few questions?

BTW: It is very easy to install it. One powershell command and a few mouse clicks: https://docs.microsoft.com/en-us/windows/wsl/install-win10

wfurt commented 6 years ago

The symlink is convenience and it does not need to exist (so as all the zones under /usr/share) The file it self has POSIX name in it and that is all what matters for time calculations.

I would be primarily concern about embedded distros like Yocto or Alpine.

BTW The CompareTimeZoneFile() can do fstat() (cheap) first and move on if file size is different. That can save on creating stream and reading any data.

find /usr/share/zoneinfo/ -type f | xargs ls -al
-r--r--r--  1 root  wheel    199 Sep 28  2016 /usr/share/zoneinfo/Pacific/Guam
-r--r--r--  1 root  wheel    250 Sep 28  2016 /usr/share/zoneinfo/Pacific/Honolulu
-r--r--r--  1 root  wheel    250 Sep 28  2016 /usr/share/zoneinfo/Pacific/Johnston
-r--r--r--  1 root  wheel    204 Sep 28  2016 /usr/share/zoneinfo/Pacific/Kiritimati
-r--r--r--  1 root  wheel    204 Sep 28  2016 /usr/share/zoneinfo/Pacific/Kosrae
-r--r--r--  1 root  wheel    211 Sep 28  2016 /usr/share/zoneinfo/Pacific/Kwajalein
-r--r--r--  1 root  wheel    171 Sep 28  2016 /usr/share/zoneinfo/Pacific/Majuro
-r--r--r--  1 root  wheel    162 Sep 28  2016 /usr/share/zoneinfo/Pacific/Marquesas
-r--r--r--  1 root  wheel    250 Sep 28  2016 /usr/share/zoneinfo/Pacific/Midway
-r--r--r--  1 root  wheel    240 Sep 28  2016 /usr/share/zoneinfo/Pacific/Nauru
-r--r--r--  1 root  wheel    200 Sep 28  2016 /usr/share/zoneinfo/Pacific/Niue
-r--r--r--  1 root  wheel    263 Sep 28  2016 /usr/share/zoneinfo/Pacific/Norfolk  
eerhardt commented 6 years ago

To fix just DateTime.Now, we could change

https://github.com/dotnet/coreclr/blob/7ab27c46cfca8b2a751316d82ea864267d1fcda4/src/mscorlib/shared/System/TimeZoneInfo.Unix.cs#L689

to not use the TimeZoneInfo.Local property, and instead mimic closer to what we do on Windows - create and cache a private local TimeZoneInfo that isn't exposed anywhere. We could just use the hard-coded "Local" value for the ID.

This wouldn't solve it for other code using TimeZoneInfo.Local, but it would solve it just for the DateTime.Now case - which is more often used.

Another thought would be to update the ID finding code to also check if tzFilePath is under GetTimeZoneDirectory(), and use the relative path under it as the ID.

These two changes can be done independently.

wfurt commented 6 years ago

BTW FreeBSD does not have symlink

[furt@toweinfu-d11 ~]$ ls -al /etc/localtime -r--r--r-- 1 root wheel 2819 Sep 27 08:37 /etc/localtime

OSX does macik:~ furt$ ls -al /etc/localtime lrwxr-xr-x 1 root wheel 39 Nov 27 06:30 /etc/localtime -> /usr/share/zoneinfo/America/Los_Angeles

wfurt commented 6 years ago

https://en.wikipedia.org/wiki/List_of_tz_database_time_zones

we can possibly fall-back to Canonical name for each POSIX zone. Zone mapping does not change much (unlike zone details)

pjanotti commented 6 years ago

Just listing a temporary workaround for WSL for anybody interested: sudo dpkg-reconfigure tzdata after that the symlink will be in place.