-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
Simplify TimeSpanFormat #102145
Simplify TimeSpanFormat #102145
Conversation
// value = 1234 => retVal = 0, valueWithoutTrailingZeros = 1234 | ||
// value = 320900 => retVal = 2, valueWithoutTrailingZeros = 3209 | ||
[MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
public static int CountDecimalTrailingZeros(uint value, out uint valueWithoutTrailingZeros) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only usage prior to change was in TimeSpanFormat.TryFormatStandard
@@ -106,39 +107,35 @@ internal enum StandardFormat | |||
g | |||
} | |||
|
|||
[MethodImpl(MethodImplOptions.AggressiveInlining)] | |||
private static void GetAbsoluteParts(TimeSpan value, out int days, out int hours, out int minutes, out int seconds, out int fraction) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried to follow pattern in DateTime.GetTime
Tagging subscribers to this area: @dotnet/area-system-datetime |
@lilinus Thank you for submitting the pull request. Typically, if a code change doesn't offer significant benefits, we aim to avoid merely cleaning up or refactoring it. While this change may pass the CI tests, there's always a chance of regression. We might consider accepting it in the future if we make other modifications to the code. Thanks once more. |
I tried to consolidate some code in TimeSpanFormat (and minor change in DateTimeFormat).
No big performance improvements but I checked for regressions. Major gain for this PR is code is simpler, and a method in common source file is moved to CoreLib, which was the only place it was used.
Benchmarks (source)