Mirror changes from dotnet/corefx#7219
Conversation
…ing and pinning (#36071) * Cleaning up the P/Invoke signatures for QPF and QPC to avoid marshalling and pinning * Responding to PR feedback. * Dropping support for low resolution stopwatch * Removing BestFitMapping=false * Removing unneeded unsafe declarations * Addressing more PR feedback * Fixing the pal_time header file * Fixing the Unix P/Invoke signatures for GetTimestamp and GetTimestampResolution to return ulong. * Fixing GetTimestampResolution to scale the result for HAVE_CLOCK_MONOTONIC Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
| return (ulong)frequency; | ||
| } | ||
| // Cache the frequency on the managed side to avoid the cost of P/Invoke on every access to Frequency | ||
| public static ulong Frequency { get; } = Interop.Sys.GetTimestampResolution(); |
There was a problem hiding this comment.
Is there any reason why this isn't shared with CoreFX instead of having the difference mechanisms we have today?
There was a problem hiding this comment.
Do you mean to move StopWatch from CoreFX to CoreLib, and make the managed ClrThreadPool depend on it? I am not sure whether it would be an improvement.
There was a problem hiding this comment.
I was more thinking of having Stopwatch in CoreFX consume HighPerformanceCounter; rather than having private QueryPerformanceFrequency and QueryPerformanceCounter methods...
There was a problem hiding this comment.
I do not have a strong opinion. There are many places where we can share a few more lines, at the cost of increased coupling. Personally, I tend to prefer less coupling.
There was a problem hiding this comment.
Fair enough. I was thinking it might be nice in the places where there is interop code involved as they are effectively "coupled" already.
It would also ensure that minor "improvements" like the signature change from out long to long* to avoid the implicit pinning on Windows apply to both places (it looks like HighPerformancCounter.Windows.cs is still using out long).
There was a problem hiding this comment.
I also don't have a strong opinion, however, as I don't think this code will get touched that often 😄
jkotas
force-pushed
the
mirror-merge-10115512
branch
from
9685f37 to
97b59b6
Compare
3 participants
This PR contains mirrored changes from dotnet/corefx
Please REBASE this PR when merging
cc @tannergooding