This repository has been archived by the owner on Nov 1, 2020. It is now read-only.
Port TimeZoneInfo cleanups from CoreCLR #2400
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TimeZoneInfo is immutable. Help enforce this by making its fields readonly.
AdjustmentRule is immutable. Help enforce this by making its fields readonly.
TransitionTime is immutable. Help enforce this by making its fields readonly.
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.
It's more efficient to concatenate the strings.
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.
|
LGTM. thanks a lot @justinvp |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contributes to #2388
Ports the following:
dotnet/coreclr#8513(this change was already in CoreRT)dotnet/coreclr#8526
dotnet/coreclr#8527
dotnet/coreclr#8528
dotnet/coreclr#8529
dotnet/coreclr#8530
dotnet/coreclr#8540
dotnet/coreclr#8575
There's one more left to port (dotnet/coreclr#8512) that I'll submit as a separate PR as it requires some minor refactoring in CoreRT to avoid having the Sort code in multiple places in CoreRT (it's only in one place in CoreCLR).
cc: @tarekgh, @DnlHarvey, @jkotas