search

NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18.12k stars 14.15k forks source link

Set TZ environment variable? #23053

Open wizeman opened 7 years ago

wizeman commented 7 years ago

Issue description

The TZ environment variable was disabled in #1463, but apparently this has performance implications:

https://blog.packagecloud.io/eng/2017/02/21/set-environment-variable-save-thousands-of-system-calls/

https://news.ycombinator.com/item?id=13697555

Steps to reproduce

See the first link above.

Technical details

rnhmjoj commented 7 years ago

On my system, with 1M cycles, the time ratio is ≈ 20/1, and the saving per syscall 1μs.

edolstra commented 7 years ago
wizeman commented 7 years ago

@edolstra: Is there any disadvantage to setting TZ=:/etc/localtime, which is what the article suggests, as long as it can be overridden?

Note that we would not be setting TZ to be a specific time zone, so we would not be reintroducing the issue that led to the removal of the definition of TZ in NixOS, we would simply define TZ to be what glibc already considers to be the default:

In the GNU C Library, the default time zone is like the specification ‘TZ=:/etc/localtime’

... except that we would set it explicitly, to prevent glibc from calling the stat() syscall every time some program calls localtime().

I could probably submit a PR myself but I'm not sure how to set this as a default for all systemd services?

wizeman commented 7 years ago

Also note that, if /etc is stored on an NFS filesystem, I can imagine stat() being a few orders of magnitude slower, having to contact the NFS server and all...

rnhmjoj commented 7 years ago

I agree with @edolstra: it's should be fixed in glibc by only accessing /etc/localtime once, as FreeBSD does.

wizeman commented 7 years ago

But if glibc only accesses /etc/localtime once, and the timezone changes, then systemd services will keep using the old timezone?

I was thinking that if we set TZ to point to the actual timezone file, then systemd services would be restarted if the timezone changes, which is probably what we want.

Either way, it's probably better to continue discussing this in #23078.

mmahut commented 5 years ago

Are there any updates to this issue, please?

rnhmjoj commented 5 years ago

So, glibc is not going to change that: a stat per localtime is required to keep the timezone info updated. Setting TZ saves the extra syscalls but it's going to break that for some programs. It seems useful since changing timezones is not super common, on servers at least. The PR needs rebasing and should be good.

stale[bot] commented 4 years ago

Thank you for your contributions.

This has been automatically marked as stale because it has had no activity for 180 days.

If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.

Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse.
  3. Ask on the #nixos channel on irc.freenode.net.