From 46b727f9c01441985333bd23dc30f034bdadef5b Mon Sep 17 00:00:00 2001 From: Justin Van Patten Date: Tue, 20 Dec 2016 10:56:16 -0800 Subject: [PATCH 1/7] Make TimeZoneInfo fields readonly TimeZoneInfo is immutable. Help enforce this by making its fields readonly. --- .../src/System/TimeZoneInfo.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs b/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs index 1f330d7231f..e0300ef6254 100644 --- a/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs +++ b/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs @@ -58,13 +58,13 @@ internal enum TimeZoneInfoResult }; // ---- SECTION: members supporting exposed properties -------------* - private String _id; - private String _displayName; - private String _standardDisplayName; - private String _daylightDisplayName; - private TimeSpan _baseUtcOffset; - private Boolean _supportsDaylightSavingTime; - private AdjustmentRule[] _adjustmentRules; + private readonly String _id; + private readonly String _displayName; + private readonly String _standardDisplayName; + private readonly String _daylightDisplayName; + private readonly TimeSpan _baseUtcOffset; + private readonly Boolean _supportsDaylightSavingTime; + private readonly AdjustmentRule[] _adjustmentRules; // constants for TimeZoneInfo.Local and TimeZoneInfo.Utc private const string c_utcId = "UTC"; From e7cda0cbd5ed1fc6902641523880f0fd76cd8f29 Mon Sep 17 00:00:00 2001 From: Justin Van Patten Date: Tue, 20 Dec 2016 10:58:12 -0800 Subject: [PATCH 2/7] Simplify TimeZoneInfo.AdjustmentRule.Equals --- src/System.Private.CoreLib/src/System/TimeZoneInfo.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs b/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs index e0300ef6254..56691dbe7f0 100644 --- a/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs +++ b/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs @@ -2699,16 +2699,13 @@ internal bool HasDaylightSaving // IEquatable public bool Equals(AdjustmentRule other) { - bool equals = (other != null + return other != null && _dateStart == other._dateStart && _dateEnd == other._dateEnd && _daylightDelta == other._daylightDelta - && _baseUtcOffsetDelta == other._baseUtcOffsetDelta); - - equals = equals && _daylightTransitionEnd.Equals(other._daylightTransitionEnd) - && _daylightTransitionStart.Equals(other._daylightTransitionStart); - - return equals; + && _baseUtcOffsetDelta == other._baseUtcOffsetDelta + && _daylightTransitionEnd.Equals(other._daylightTransitionEnd) + && _daylightTransitionStart.Equals(other._daylightTransitionStart); } From 8cd33d38c2efc18389f2b3577b6b053a69d4fbe7 Mon Sep 17 00:00:00 2001 From: Justin Van Patten Date: Tue, 20 Dec 2016 11:06:23 -0800 Subject: [PATCH 3/7] Make TimeZoneInfo.AdjustmentRule fields readonly AdjustmentRule is immutable. Help enforce this by making its fields readonly. --- .../src/System/TimeZoneInfo.cs | 61 ++++++++++++------- 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs b/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs index 56691dbe7f0..1d68b9e880d 100644 --- a/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs +++ b/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs @@ -2625,12 +2625,12 @@ private static bool TransitionTimeFromTimeZoneInformation(TIME_ZONE_INFORMATION sealed public class AdjustmentRule : IEquatable, ISerializable, IDeserializationCallback { // ---- SECTION: members supporting exposed properties -------------* - private DateTime _dateStart; - private DateTime _dateEnd; - private TimeSpan _daylightDelta; - private TransitionTime _daylightTransitionStart; - private TransitionTime _daylightTransitionEnd; - private TimeSpan _baseUtcOffsetDelta; // delta from the default Utc offset (utcOffset = defaultUtcOffset + _baseUtcOffsetDelta) + private readonly DateTime _dateStart; + private readonly DateTime _dateEnd; + private readonly TimeSpan _daylightDelta; + private readonly TransitionTime _daylightTransitionStart; + private readonly TransitionTime _daylightTransitionEnd; + private readonly TimeSpan _baseUtcOffsetDelta; // delta from the default Utc offset (utcOffset = defaultUtcOffset + _baseUtcOffsetDelta) // ---- SECTION: public properties --------------* @@ -2718,7 +2718,24 @@ public override int GetHashCode() // -------- SECTION: constructors -----------------* - private AdjustmentRule() { } + private AdjustmentRule( + DateTime dateStart, + DateTime dateEnd, + TimeSpan daylightDelta, + TransitionTime daylightTransitionStart, + TransitionTime daylightTransitionEnd, + TimeSpan baseUtcOffsetDelta) + { + ValidateAdjustmentRule(dateStart, dateEnd, daylightDelta, + daylightTransitionStart, daylightTransitionEnd); + + _dateStart = dateStart; + _dateEnd = dateEnd; + _daylightDelta = daylightDelta; + _daylightTransitionStart = daylightTransitionStart; + _daylightTransitionEnd = daylightTransitionEnd; + _baseUtcOffsetDelta = baseUtcOffsetDelta; + } // -------- SECTION: factory methods -----------------* @@ -2730,19 +2747,13 @@ public static AdjustmentRule CreateAdjustmentRule( TransitionTime daylightTransitionStart, TransitionTime daylightTransitionEnd) { - ValidateAdjustmentRule(dateStart, dateEnd, daylightDelta, - daylightTransitionStart, daylightTransitionEnd); - - AdjustmentRule rule = new AdjustmentRule(); - - rule._dateStart = dateStart; - rule._dateEnd = dateEnd; - rule._daylightDelta = daylightDelta; - rule._daylightTransitionStart = daylightTransitionStart; - rule._daylightTransitionEnd = daylightTransitionEnd; - rule._baseUtcOffsetDelta = TimeSpan.Zero; - - return rule; + return new AdjustmentRule( + dateStart, + dateEnd, + daylightDelta, + daylightTransitionStart, + daylightTransitionEnd, + baseUtcOffsetDelta: TimeSpan.Zero); } internal static AdjustmentRule CreateAdjustmentRule( @@ -2753,9 +2764,13 @@ internal static AdjustmentRule CreateAdjustmentRule( TransitionTime daylightTransitionEnd, TimeSpan baseUtcOffsetDelta) { - AdjustmentRule rule = CreateAdjustmentRule(dateStart, dateEnd, daylightDelta, daylightTransitionStart, daylightTransitionEnd); - rule._baseUtcOffsetDelta = baseUtcOffsetDelta; - return rule; + return new AdjustmentRule( + dateStart, + dateEnd, + daylightDelta, + daylightTransitionStart, + daylightTransitionEnd, + baseUtcOffsetDelta); } // ----- SECTION: internal utility methods ----------------* From 3ec6ffbf6f025acd3c29dd59d351cac22452317b Mon Sep 17 00:00:00 2001 From: Justin Van Patten Date: Tue, 20 Dec 2016 11:10:00 -0800 Subject: [PATCH 4/7] Make TimeZoneInfo.TransitionTime fields readonly TransitionTime is immutable. Help enforce this by making its fields readonly. --- .../src/System/TimeZoneInfo.cs | 66 ++++++++----------- 1 file changed, 26 insertions(+), 40 deletions(-) diff --git a/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs b/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs index 1d68b9e880d..64930dcf55f 100644 --- a/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs +++ b/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs @@ -2934,12 +2934,12 @@ private AdjustmentRule(SerializationInfo info, StreamingContext context) public struct TransitionTime : IEquatable, ISerializable, IDeserializationCallback { // ---- SECTION: members supporting exposed properties -------------* - private DateTime _timeOfDay; - private byte _month; - private byte _week; - private byte _day; - private DayOfWeek _dayOfWeek; - private Boolean _isFixedDateRule; + private readonly DateTime _timeOfDay; + private readonly byte _month; + private readonly byte _week; + private readonly byte _day; + private readonly DayOfWeek _dayOfWeek; + private readonly Boolean _isFixedDateRule; // ---- SECTION: public properties --------------* @@ -3043,16 +3043,24 @@ public override int GetHashCode() // -------- SECTION: constructors -----------------* - /* - private TransitionTime() { - _timeOfDay = new DateTime(); - _month = 0; - _week = 0; - _day = 0; - _dayOfWeek = DayOfWeek.Sunday; - _isFixedDateRule = false; - } - */ + + private TransitionTime( + DateTime timeOfDay, + Int32 month, + Int32 week, + Int32 day, + DayOfWeek dayOfWeek, + Boolean isFixedDateRule) + { + ValidateTransitionTime(timeOfDay, month, week, day, dayOfWeek); + + _timeOfDay = timeOfDay; + _month = (byte)month; + _week = (byte)week; + _day = (byte)day; + _dayOfWeek = dayOfWeek; + _isFixedDateRule = isFixedDateRule; + } // -------- SECTION: factory methods -----------------* @@ -3063,7 +3071,7 @@ public static TransitionTime CreateFixedDateRule( Int32 month, Int32 day) { - return CreateTransitionTime(timeOfDay, month, 1, day, DayOfWeek.Sunday, true); + return new TransitionTime(timeOfDay, month, 1, day, DayOfWeek.Sunday, true); } @@ -3073,29 +3081,7 @@ public static TransitionTime CreateFloatingDateRule( Int32 week, DayOfWeek dayOfWeek) { - return CreateTransitionTime(timeOfDay, month, week, 1, dayOfWeek, false); - } - - - private static TransitionTime CreateTransitionTime( - DateTime timeOfDay, - Int32 month, - Int32 week, - Int32 day, - DayOfWeek dayOfWeek, - Boolean isFixedDateRule) - { - ValidateTransitionTime(timeOfDay, month, week, day, dayOfWeek); - - TransitionTime t = new TransitionTime(); - t._isFixedDateRule = isFixedDateRule; - t._timeOfDay = timeOfDay; - t._dayOfWeek = dayOfWeek; - t._day = (byte)day; - t._week = (byte)week; - t._month = (byte)month; - - return t; + return new TransitionTime(timeOfDay, month, week, 1, dayOfWeek, false); } From c6fc252e45692d9a7ea5fccf42c02d505d7158e1 Mon Sep 17 00:00:00 2001 From: Justin Van Patten Date: Tue, 20 Dec 2016 11:18:11 -0800 Subject: [PATCH 5/7] Preallocate the TimeZoneInfo.Utc instance There doesn't appear to be a good reason why the TimeZoneInfo.Utc instance needs to be cleared when TimeZoneInfo.ClearCachedData() is called. Instead, we can pre-allocate and reuse a singleton instance, obviating the need for the lazy-initialization/locking mechanics. --- .../src/System/TimeZoneInfo.cs | 54 +++++-------------- 1 file changed, 13 insertions(+), 41 deletions(-) diff --git a/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs b/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs index 64930dcf55f..e6c5723a769 100644 --- a/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs +++ b/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs @@ -70,6 +70,8 @@ internal enum TimeZoneInfoResult private const string c_utcId = "UTC"; private const string c_localId = "Local"; + private static readonly TimeZoneInfo s_utcTimeZone = CreateCustomTimeZone(c_utcId, TimeSpan.Zero, c_utcId, c_utcId); + private static CachedData s_cachedData = new CachedData(); // @@ -82,7 +84,6 @@ internal enum TimeZoneInfoResult private class CachedData { private volatile TimeZoneInfo _localTimeZone; - private volatile TimeZoneInfo _utcTimeZone; private Lock _lock = new Lock(); private TimeZoneInfo CreateLocal() @@ -125,35 +126,6 @@ public TimeZoneInfo Local } } - private TimeZoneInfo CreateUtc() - { - using (LockHolder.Hold(_lock)) - { - TimeZoneInfo timeZone = _utcTimeZone; - if (timeZone == null) - { - timeZone = CreateCustomTimeZone(c_utcId, TimeSpan.Zero, c_utcId, c_utcId); - _utcTimeZone = timeZone; - } - return timeZone; - } - } - - public TimeZoneInfo Utc - { - get - { - Contract.Ensures(Contract.Result() != null); - - TimeZoneInfo timeZone = _utcTimeZone; - if (timeZone == null) - { - timeZone = CreateUtc(); - } - return timeZone; - } - } - // // GetCorrespondingKind- // @@ -182,7 +154,7 @@ public DateTimeKind GetCorrespondingKind(TimeZoneInfo timeZone) // in this example. Only when the user passes in TimeZoneInfo.Local or // TimeZoneInfo.Utc to the ConvertTime(...) methods will this check succeed. // - if ((object)timeZone == (object)_utcTimeZone) + if ((object)timeZone == (object)s_utcTimeZone) { kind = DateTimeKind.Utc; } @@ -438,7 +410,7 @@ public TimeSpan[] GetAmbiguousTimeOffsets(DateTime dateTime) else if (dateTime.Kind == DateTimeKind.Utc) { CachedData cachedData = s_cachedData; - adjustedTime = TimeZoneInfo.ConvertTime(dateTime, cachedData.Utc, this, TimeZoneInfoOptions.None, cachedData); + adjustedTime = TimeZoneInfo.ConvertTime(dateTime, s_utcTimeZone, this, TimeZoneInfoOptions.None, cachedData); } else { @@ -514,7 +486,7 @@ private TimeSpan GetUtcOffset(DateTime dateTime, TimeZoneInfoOptions flags, Cach // // normal case of converting from Local to Utc and then getting the offset from the UTC DateTime // - DateTime adjustedTime = TimeZoneInfo.ConvertTime(dateTime, cachedData.Local, cachedData.Utc, flags); + DateTime adjustedTime = TimeZoneInfo.ConvertTime(dateTime, cachedData.Local, s_utcTimeZone, flags); return GetUtcOffsetFromUtc(adjustedTime, this); } // @@ -590,7 +562,7 @@ internal Boolean IsAmbiguousTime(DateTime dateTime, TimeZoneInfoOptions flags) else if (dateTime.Kind == DateTimeKind.Utc) { CachedData cachedData = s_cachedData; - adjustedTime = TimeZoneInfo.ConvertTime(dateTime, cachedData.Utc, this, flags, cachedData); + adjustedTime = TimeZoneInfo.ConvertTime(dateTime, s_utcTimeZone, this, flags, cachedData); } else { @@ -772,7 +744,7 @@ public static DateTime ConvertTimeBySystemTimeZoneId(DateTime dateTime, String s // be reference equal to the new TimeZoneInfo.Utc // CachedData cachedData = s_cachedData; - return ConvertTime(dateTime, cachedData.Utc, FindSystemTimeZoneById(destinationTimeZoneId), TimeZoneInfoOptions.None, cachedData); + return ConvertTime(dateTime, s_utcTimeZone, FindSystemTimeZoneById(destinationTimeZoneId), TimeZoneInfoOptions.None, cachedData); } else { @@ -832,7 +804,7 @@ public static DateTime ConvertTime(DateTime dateTime, TimeZoneInfo destinationTi CachedData cachedData = s_cachedData; if (dateTime.Kind == DateTimeKind.Utc) { - return ConvertTime(dateTime, cachedData.Utc, destinationTimeZone, TimeZoneInfoOptions.None, cachedData); + return ConvertTime(dateTime, s_utcTimeZone, destinationTimeZone, TimeZoneInfoOptions.None, cachedData); } else { @@ -937,7 +909,7 @@ private static DateTime ConvertTime(DateTime dateTime, TimeZoneInfo sourceTimeZo public static DateTime ConvertTimeFromUtc(DateTime dateTime, TimeZoneInfo destinationTimeZone) { CachedData cachedData = s_cachedData; - return ConvertTime(dateTime, cachedData.Utc, destinationTimeZone, TimeZoneInfoOptions.None, cachedData); + return ConvertTime(dateTime, s_utcTimeZone, destinationTimeZone, TimeZoneInfoOptions.None, cachedData); } // @@ -952,7 +924,7 @@ public static DateTime ConvertTimeToUtc(DateTime dateTime) return dateTime; } CachedData cachedData = s_cachedData; - return ConvertTime(dateTime, cachedData.Local, cachedData.Utc, TimeZoneInfoOptions.None, cachedData); + return ConvertTime(dateTime, cachedData.Local, s_utcTimeZone, TimeZoneInfoOptions.None, cachedData); } internal static DateTime ConvertTimeToUtc(DateTime dateTime, TimeZoneInfoOptions flags) @@ -962,13 +934,13 @@ internal static DateTime ConvertTimeToUtc(DateTime dateTime, TimeZoneInfoOptions return dateTime; } CachedData cachedData = s_cachedData; - return ConvertTime(dateTime, cachedData.Local, cachedData.Utc, flags, cachedData); + return ConvertTime(dateTime, cachedData.Local, s_utcTimeZone, flags, cachedData); } public static DateTime ConvertTimeToUtc(DateTime dateTime, TimeZoneInfo sourceTimeZone) { CachedData cachedData = s_cachedData; - return ConvertTime(dateTime, sourceTimeZone, cachedData.Utc, TimeZoneInfoOptions.None, cachedData); + return ConvertTime(dateTime, sourceTimeZone, s_utcTimeZone, TimeZoneInfoOptions.None, cachedData); } // @@ -1123,7 +1095,7 @@ public static TimeZoneInfo Utc get { Contract.Ensures(Contract.Result() != null); - return s_cachedData.Utc; + return s_utcTimeZone; } } From 18e7b72ccdc975bb58923067c76b2af8e040f0f8 Mon Sep 17 00:00:00 2001 From: Justin Van Patten Date: Tue, 20 Dec 2016 11:24:19 -0800 Subject: [PATCH 6/7] TimeZoneInfo: Use string.Concat instead of string.Format It's more efficient to concatenate the strings. --- .../src/System/TimeZoneInfo.Win32.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/System.Private.CoreLib/src/System/TimeZoneInfo.Win32.cs b/src/System.Private.CoreLib/src/System/TimeZoneInfo.Win32.cs index 4ca2c01e388..69ad37dcd90 100644 --- a/src/System.Private.CoreLib/src/System/TimeZoneInfo.Win32.cs +++ b/src/System.Private.CoreLib/src/System/TimeZoneInfo.Win32.cs @@ -610,8 +610,7 @@ private static bool TryCreateAdjustmentRules(string id, REGISTRY_TIME_ZONE_INFOR try { using (RegistryKey dynamicKey = RegistryKey.GetBaseKey(RegistryKey.HKEY_LOCAL_MACHINE).OpenSubKey( - String.Format(CultureInfo.InvariantCulture, "{0}\\{1}\\Dynamic DST", - c_timeZonesRegistryHive, id), + c_timeZonesRegistryHive + "\\" + id + "\\Dynamic DST", false )) { @@ -788,8 +787,7 @@ static unsafe private Boolean TryCompareTimeZoneInformationToRegistry(TIME_ZONE_ { dstDisabled = false; using (RegistryKey key = RegistryKey.GetBaseKey(RegistryKey.HKEY_LOCAL_MACHINE).OpenSubKey( - String.Format(CultureInfo.InvariantCulture, "{0}\\{1}", - c_timeZonesRegistryHive, id), + c_timeZonesRegistryHive + "\\" + id, false )) { @@ -1063,8 +1061,7 @@ private static TimeZoneInfoResult TryGetTimeZoneByRegistryKey(string id, out Tim e = null; using (RegistryKey key = RegistryKey.GetBaseKey(RegistryKey.HKEY_LOCAL_MACHINE).OpenSubKey( - String.Format(CultureInfo.InvariantCulture, "{0}\\{1}", - c_timeZonesRegistryHive, id), + c_timeZonesRegistryHive + "\\" + id, false )) { From 1a3e4d69e3ba4a3195ed2c756e60076f8c5409e7 Mon Sep 17 00:00:00 2001 From: Justin Van Patten Date: Tue, 20 Dec 2016 11:51:16 -0800 Subject: [PATCH 7/7] TimeZoneInfo: Avoid cloning privately-created ArgumentRule[] arrays TimeZoneInfo currently always creates a defensive copy of the specified ArgumentRule[] array when created. This makes sense for the public static factory methods. However, there's no need to create a defensive copy of arrays created privately as part of its implementation (e.g. reading the rules from the registry/disk). This change avoids the unnecessary cloning. --- .../src/System/TimeZoneInfo.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs b/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs index e6c5723a769..4917953d127 100644 --- a/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs +++ b/src/System.Private.CoreLib/src/System/TimeZoneInfo.cs @@ -1137,7 +1137,7 @@ public static TimeZoneInfo CreateCustomTimeZone( String daylightDisplayName, AdjustmentRule[] adjustmentRules) { - return new TimeZoneInfo( + return CreateCustomTimeZone( id, baseUtcOffset, displayName, @@ -1165,6 +1165,11 @@ public static TimeZoneInfo CreateCustomTimeZone( AdjustmentRule[] adjustmentRules, Boolean disableDaylightSavingTime) { + if (!disableDaylightSavingTime && adjustmentRules?.Length > 0) + { + adjustmentRules = (AdjustmentRule[])adjustmentRules.Clone(); + } + return new TimeZoneInfo( id, baseUtcOffset, @@ -2256,17 +2261,13 @@ private TimeZoneInfo( Boolean adjustmentRulesSupportDst; ValidateTimeZoneInfo(id, baseUtcOffset, adjustmentRules, out adjustmentRulesSupportDst); - if (!disableDaylightSavingTime && adjustmentRules != null && adjustmentRules.Length > 0) - { - _adjustmentRules = (AdjustmentRule[])adjustmentRules.Clone(); - } - _id = id; _baseUtcOffset = baseUtcOffset; _displayName = displayName; _standardDisplayName = standardDisplayName; _daylightDisplayName = (disableDaylightSavingTime ? null : daylightDisplayName); _supportsDaylightSavingTime = adjustmentRulesSupportDst && !disableDaylightSavingTime; + _adjustmentRules = adjustmentRules; } // @@ -3281,7 +3282,7 @@ public static TimeZoneInfo GetDeserializedTimeZoneInfo(String source) try { - return TimeZoneInfo.CreateCustomTimeZone(id, baseUtcOffset, displayName, standardName, daylightName, rules); + return new TimeZoneInfo(id, baseUtcOffset, displayName, standardName, daylightName, rules, disableDaylightSavingTime: false); } catch (ArgumentException ex) {