Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libs][Unix][perf] Lazily initialize TimeZoneInfo names and order GetSystemTimeZones by Ids #88368

Merged
merged 17 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Globalization;
using System.Threading;

namespace System
{
Expand All @@ -21,24 +22,7 @@ public sealed partial class TimeZoneInfo
"Pacific/Pitcairn" // Prefer "Pitcairn Islands Time" over "Pitcairn Time"
};

// Main function that is called during construction to populate the three display names
private static void TryPopulateTimeZoneDisplayNamesFromGlobalizationData(string timeZoneId, TimeSpan baseUtcOffset, ref string? standardDisplayName, ref string? daylightDisplayName, ref string? displayName)
{
if (GlobalizationMode.Invariant)
{
return;
}

// Determine the culture to use
CultureInfo uiCulture = CultureInfo.CurrentUICulture;
if (uiCulture.Name.Length == 0)
uiCulture = CultureInfo.GetCultureInfo(FallbackCultureName); // ICU doesn't work nicely with InvariantCulture

// Attempt to populate the fields backing the StandardName, DaylightName, and DisplayName from globalization data.
GetDisplayName(timeZoneId, Interop.Globalization.TimeZoneDisplayNameType.Standard, uiCulture.Name, ref standardDisplayName);
GetDisplayName(timeZoneId, Interop.Globalization.TimeZoneDisplayNameType.DaylightSavings, uiCulture.Name, ref daylightDisplayName);
GetFullValueForDisplayNameField(timeZoneId, baseUtcOffset, uiCulture, ref displayName);
}
private static CultureInfo? _uiCulture;

// Helper function to get the standard display name for the UTC static time zone instance
private static string GetUtcStandardDisplayName()
Expand Down Expand Up @@ -67,6 +51,34 @@ private static string GetUtcFullDisplayName(string timeZoneId, string standardDi
}
#pragma warning restore IDE0060

private static CultureInfo UICulture
{
get
{
if (_uiCulture == null)
{
// Determine the culture to use
mdh1418 marked this conversation as resolved.
Show resolved Hide resolved
CultureInfo uiCulture = CultureInfo.CurrentUICulture;
if (uiCulture.Name.Length == 0)
uiCulture = CultureInfo.GetCultureInfo(FallbackCultureName); // ICU doesn't work nicely with InvariantCulture

Interlocked.CompareExchange(ref _uiCulture, uiCulture, null);
}

return _uiCulture;
}
}

private static void GetStandardDisplayName(string timeZoneId, ref string? displayName)
{
GetDisplayName(timeZoneId, Interop.Globalization.TimeZoneDisplayNameType.Standard, UICulture.Name, ref displayName);
}

private static void GetDaylightDisplayName(string timeZoneId, ref string? displayName)
{
GetDisplayName(timeZoneId, Interop.Globalization.TimeZoneDisplayNameType.DaylightSavings, UICulture.Name, ref displayName);
}

// Helper function that retrieves various forms of time zone display names from ICU
private static unsafe void GetDisplayName(string timeZoneId, Interop.Globalization.TimeZoneDisplayNameType nameType, string uiCulture, ref string? displayName)
{
Expand Down Expand Up @@ -115,14 +127,15 @@ private static unsafe void GetDisplayName(string timeZoneId, Interop.Globalizati
}

// Helper function that builds the value backing the DisplayName field from globalization data.
private static void GetFullValueForDisplayNameField(string timeZoneId, TimeSpan baseUtcOffset, CultureInfo uiCulture, ref string? displayName)
private static void GetFullValueForDisplayNameField(string timeZoneId, TimeSpan baseUtcOffset, ref string? displayName)
{
// There are a few diffent ways we might show the display name depending on the data.
// The algorithm used below should avoid duplicating the same words while still achieving the
// goal of providing a unique, discoverable, and intuitive name.

// Try to get the generic name for this time zone.
string? genericName = null;
CultureInfo uiCulture = UICulture;
GetDisplayName(timeZoneId, Interop.Globalization.TimeZoneDisplayNameType.Generic, uiCulture.Name, ref genericName);
if (genericName == null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ namespace System
public sealed partial class TimeZoneInfo
{
#pragma warning disable IDE0060
static partial void TryPopulateTimeZoneDisplayNamesFromGlobalizationData(string timeZoneId, TimeSpan baseUtcOffset, ref string? standardDisplayName, ref string? daylightDisplayName, ref string? displayName);
static partial void GetFullValueForDisplayNameField(string timeZoneId, TimeSpan baseUtcOffset, ref string? displayName);

static partial void GetStandardDisplayName(string timeZoneId, ref string? displayName);

static partial void GetDaylightDisplayName(string timeZoneId, ref string? displayName);

private static string GetUtcStandardDisplayName()
{
Expand Down
115 changes: 80 additions & 35 deletions src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,48 +18,58 @@ public sealed partial class TimeZoneInfo
{
private const string DefaultTimeZoneDirectory = "/usr/share/zoneinfo/";

// UTC aliases per https://github.com/unicode-org/cldr/blob/master/common/bcp47/timezone.xml
// Set fallback values using abbreviations, base offset, and id
// These are expected in environments without time zone globalization data
private string? _standardAbbrevName;
private string? _daylightAbbrevName;

// Handle UTC and its aliases per https://github.com/unicode-org/cldr/blob/master/common/bcp47/timezone.xml
// Hard-coded because we need to treat all aliases of UTC the same even when globalization data is not available.
// (This list is not likely to change.)
private static readonly string[] s_UtcAliases = new[] {
"Etc/UTC",
"Etc/UCT",
"Etc/Universal",
"Etc/Zulu",
"UCT",
"UTC",
"Universal",
"Zulu"
};
private static bool IsUtcAlias (string id)
{
switch ((ushort)id[0])
{
case 69: // e
case 101: // E
return string.Equals(id, "Etc/UTC", StringComparison.OrdinalIgnoreCase) ||
string.Equals(id, "Etc/Universal", StringComparison.OrdinalIgnoreCase) ||
string.Equals(id, "Etc/UTC", StringComparison.OrdinalIgnoreCase) ||
string.Equals(id, "Etc/Zulu", StringComparison.OrdinalIgnoreCase);
case 85: // u
case 117: // U
return string.Equals(id, "UCT", StringComparison.OrdinalIgnoreCase) ||
string.Equals(id, "UTC", StringComparison.OrdinalIgnoreCase) ||
string.Equals(id, "Universal", StringComparison.OrdinalIgnoreCase);
case 90: // z
case 122: // Z
return string.Equals(id, "Zulu", StringComparison.OrdinalIgnoreCase);
}

return false;
}

private TimeZoneInfo(byte[] data, string id, bool dstDisabled)
{
_id = id;

HasIanaId = true;

// Handle UTC and its aliases
if (StringArrayContains(_id, s_UtcAliases, StringComparison.OrdinalIgnoreCase))
if (IsUtcAlias(id))
{
_standardDisplayName = GetUtcStandardDisplayName();
_daylightDisplayName = _standardDisplayName;
_displayName = GetUtcFullDisplayName(_id, _standardDisplayName);
_baseUtcOffset = TimeSpan.Zero;
_adjustmentRules = Array.Empty<AdjustmentRule>();
return;
}

TZifHead t;
DateTime[] dts;
byte[] typeOfLocalTime;
TZifType[] transitionType;
string zoneAbbreviations;
string? futureTransitionsPosixFormat;
string? standardAbbrevName = null;
string? daylightAbbrevName = null;

// parse the raw TZif bytes; this method can throw ArgumentException when the data is malformed.
TZif_ParseRaw(data, out t, out dts, out typeOfLocalTime, out transitionType, out zoneAbbreviations, out futureTransitionsPosixFormat);
TZif_ParseRaw(data, out dts, out typeOfLocalTime, out transitionType, out zoneAbbreviations, out futureTransitionsPosixFormat);

// find the best matching baseUtcOffset and display strings based on the current utcNow value.
// NOTE: read the Standard and Daylight display strings from the tzfile now in case they can't be loaded later
Expand All @@ -71,11 +81,11 @@ private TimeZoneInfo(byte[] data, string id, bool dstDisabled)
if (!transitionType[type].IsDst)
{
_baseUtcOffset = transitionType[type].UtcOffset;
standardAbbrevName = TZif_GetZoneAbbreviation(zoneAbbreviations, transitionType[type].AbbreviationIndex);
_standardAbbrevName = TZif_GetZoneAbbreviation(zoneAbbreviations, transitionType[type].AbbreviationIndex);
}
else
{
daylightAbbrevName = TZif_GetZoneAbbreviation(zoneAbbreviations, transitionType[type].AbbreviationIndex);
_daylightAbbrevName = TZif_GetZoneAbbreviation(zoneAbbreviations, transitionType[type].AbbreviationIndex);
}
}

Expand All @@ -88,24 +98,15 @@ private TimeZoneInfo(byte[] data, string id, bool dstDisabled)
if (!transitionType[i].IsDst)
{
_baseUtcOffset = transitionType[i].UtcOffset;
standardAbbrevName = TZif_GetZoneAbbreviation(zoneAbbreviations, transitionType[i].AbbreviationIndex);
_standardAbbrevName = TZif_GetZoneAbbreviation(zoneAbbreviations, transitionType[i].AbbreviationIndex);
}
else
{
daylightAbbrevName = TZif_GetZoneAbbreviation(zoneAbbreviations, transitionType[i].AbbreviationIndex);
_daylightAbbrevName = TZif_GetZoneAbbreviation(zoneAbbreviations, transitionType[i].AbbreviationIndex);
}
}
}

// Set fallback values using abbreviations, base offset, and id
// These are expected in environments without time zone globalization data
_standardDisplayName = standardAbbrevName;
_daylightDisplayName = daylightAbbrevName ?? standardAbbrevName;
_displayName = string.Create(null, stackalloc char[256], $"(UTC{(_baseUtcOffset >= TimeSpan.Zero ? '+' : '-')}{_baseUtcOffset:hh\\:mm}) {_id}");

// Try to populate the display names from the globalization data
TryPopulateTimeZoneDisplayNamesFromGlobalizationData(_id, _baseUtcOffset, ref _standardDisplayName, ref _daylightDisplayName, ref _displayName);
mdh1418 marked this conversation as resolved.
Show resolved Hide resolved

// TZif supports seconds-level granularity with offsets but TimeZoneInfo only supports minutes since it aligns
// with DateTimeOffset, SQL Server, and the W3C XML Specification
if (_baseUtcOffset.Ticks % TimeSpan.TicksPerMinute != 0)
Expand Down Expand Up @@ -219,6 +220,50 @@ public AdjustmentRule[] GetAdjustmentRules()
return rulesList.ToArray();
}

private string? PopulateDisplayName()
{
if (IsUtcAlias(Id))
return GetUtcFullDisplayName(Id, StandardName);

// Set fallback value using abbreviations, base offset, and id
// These are expected in environments without time zone globalization data
string? displayName = string.Create(null, stackalloc char[256], $"(UTC{(_baseUtcOffset >= TimeSpan.Zero ? '+' : '-')}{_baseUtcOffset:hh\\:mm}) {_id}");
if (GlobalizationMode.Invariant)
return displayName;

GetFullValueForDisplayNameField(Id, BaseUtcOffset, ref displayName);

return displayName;
}

private string? PopulateStandardDisplayName()
{
if (IsUtcAlias(Id))
return GetUtcStandardDisplayName();

string? standardDisplayName = _standardAbbrevName;
if (GlobalizationMode.Invariant)
return standardDisplayName;

GetStandardDisplayName(Id, ref standardDisplayName);

return standardDisplayName;
}

private string? PopulateDaylightDisplayName()
{
if (IsUtcAlias(Id))
return StandardName;

string? daylightDisplayName = _daylightAbbrevName ?? _standardAbbrevName;
if (GlobalizationMode.Invariant)
return daylightDisplayName;

GetDaylightDisplayName(Id, ref daylightDisplayName);

return daylightDisplayName;
}

private static void PopulateAllSystemTimeZones(CachedData cachedData)
{
Debug.Assert(Monitor.IsEntered(cachedData));
Expand Down Expand Up @@ -1065,15 +1110,15 @@ private static DateTime TZif_UnixTimeToDateTime(long unixTime) =>
unixTime > DateTimeOffset.UnixMaxSeconds ? DateTime.MaxValue :
DateTimeOffset.FromUnixTimeSeconds(unixTime).UtcDateTime;

private static void TZif_ParseRaw(byte[] data, out TZifHead t, out DateTime[] dts, out byte[] typeOfLocalTime, out TZifType[] transitionType,
private static void TZif_ParseRaw(byte[] data, out DateTime[] dts, out byte[] typeOfLocalTime, out TZifType[] transitionType,
out string zoneAbbreviations, out string? futureTransitionsPosixFormat)
{
futureTransitionsPosixFormat = null;

// read in the 44-byte TZ header containing the count/length fields
//
int index = 0;
t = new TZifHead(data, index);
TZifHead t = new TZifHead(data, index);
index += TZifHead.Length;

int timeValuesLength = 4; // the first version uses 4-bytes to specify times
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,24 @@ public AdjustmentRule[] GetAdjustmentRules()
return (AdjustmentRule[])_adjustmentRules.Clone();
}

private static string? PopulateDisplayName()
{
// Keep window's implementation to populate via constructor
return null;
}

private static string? PopulateStandardDisplayName()
{
// Keep window's implementation to populate via constructor
return null;
}

private static string? PopulateDaylightDisplayName()
{
// Keep window's implementation to populate via constructor
return null;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would these ever be called on Windows? if not, then at least assert or throw exception here to catch any future wrong use of it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there could be cases where _displayName is still null, like when using the public APIs CreateCustomTimeZone. So if for some reason someone used TimeZoneInfo myTzi = CreateCustomTimeZone(<id>, <offset>, null, null, null, <adjustmentRules>) and later invoked myTzi.DisplayName, this would be hit.

I think in this case, we should honor that the internal fields have not been set and still return null?
But thats a good point.... I wouldn't want TimeZoneInfo objects created by CreateCustomTimeZone to have their fields unexpectedly populated (and incur the performance hit) if they intended for them to have a null value.

Would the best way to check that be by having some sort of boolean to track whether or not it had been set as null on purpose?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that CreateCustomTimeZone sets the names to string.Empty if they were null, these shouldn't be hit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added Debug.Assert(false); in the window's implementation of these methods.

I modified the primary constructor in TimeZoneInfo.cs to have ?? string.Empty for the internal fields (and removed the modifications from CreateCustomTimeZone as they will eventually flow to this constructor.

private TimeZoneInfo(
                string id,
                TimeSpan baseUtcOffset,
                string? displayName,
                string? standardDisplayName,
                string? daylightDisplayName,
                AdjustmentRule[]? adjustmentRules,
                bool disableDaylightSavingTime,
                bool hasIanaId = false)

In doing so, I believe for windows the only other TimeZoneInfo constructor that is of interest is

private TimeZoneInfo(in TIME_ZONE_INFORMATION zone, bool dstDisabled)
, which sets the fields to non null values, so we should not hit these Populate* methods on Windows (unless I'm missing something else)

Moreover, this doesn't get used anywhere does it?

private TimeZoneInfo(SerializationInfo info, StreamingContext context)
{
ArgumentNullException.ThrowIfNull(info);
_id = (string)info.GetValue("Id", typeof(string))!; // Do not rename (binary serialization)
_displayName = (string?)info.GetValue("DisplayName", typeof(string)); // Do not rename (binary serialization)
_standardDisplayName = (string?)info.GetValue("StandardName", typeof(string)); // Do not rename (binary serialization)
_daylightDisplayName = (string?)info.GetValue("DaylightName", typeof(string)); // Do not rename (binary serialization)
_baseUtcOffset = (TimeSpan)info.GetValue("BaseUtcOffset", typeof(TimeSpan))!; // Do not rename (binary serialization)
_adjustmentRules = (AdjustmentRule[]?)info.GetValue("AdjustmentRules", typeof(AdjustmentRule[])); // Do not rename (binary serialization)
_supportsDaylightSavingTime = (bool)info.GetValue("SupportsDaylightSavingTime", typeof(bool))!; // Do not rename (binary serialization)
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't get used anywhere does it?

I recall it can be used by some serialization engines like Data Contract serialization. Please keep it for compat.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah even though it is a private constructor and isn't referenced in the TimeZoneInfo.*.cs?

private static void PopulateAllSystemTimeZones(CachedData cachedData)
{
Debug.Assert(Monitor.IsEntered(cachedData));
Expand Down
Loading