Do some cleanup on TimeSpan to ensure better inlining and clearer semantics#94226
Do some cleanup on TimeSpan to ensure better inlining and clearer semantics#94226tannergooding merged 4 commits intodotnet:mainfrom
Conversation
ghost
added
the
needs-area-label
tannergooding
force-pushed
the
timespan-simple
branch
from
f9abbc6 to
c687355
Compare
|
Some of this will allow constants to be folded for #94143 if we went with the following implementation: public static TimeSpan FromHours(int hours, long minutes = 0, long seconds = 0, long milliseconds = 0, long microseconds = 0)
{
return FromHours(hours)
+ FromMinutes(minutes)
+ FromSeconds(seconds)
+ FromMilliseconds(milliseconds)
+ FromMicroseconds(microseconds);
} |
tannergooding
added
area-System.DateTime
and removed
needs-area-label
|
Tagging subscribers to this area: @dotnet/area-system-datetime Issue DetailsAs per the title, this does some cleanup of TimeSpan to more consistently use throw helpers, to more consistently use named constants for important values, and to ensure that the implementation lives in the operators so less inlining is needed in the common case.
|
tarekgh
left a comment
tarekgh
left a comment
There was a problem hiding this comment.
LGTM. thanks for cleaning this up!
|
|
||
| if ((t1Sign == (t2._ticks >> 63)) && (t1Sign != (result >> 63))) | ||
| { | ||
| // Overflow if signs of operands was identical and result's sign was opposite. |
There was a problem hiding this comment.
just curious, you could have used >>> instead of >> here, right? then it would only be the low order bit holding the sign. does it make any difference to perf?
There was a problem hiding this comment.
Signed and unsigned shift have the same cost in x64 and Arm64 hardware, so no real difference
ghost
locked as resolved and limited conversation to collaborators
3 participants
As per the title, this does some cleanup of TimeSpan to more consistently use throw helpers, to more consistently use named constants for important values, and to ensure that the implementation lives in the operators so less inlining is needed in the common case.