Use deterministic Process.StartTime for OS X #39434#39572
Use deterministic Process.StartTime for OS X #39434#39572stephentoub merged 8 commits intodotnet:masterfrom
Conversation
|
Thanks. Whatever the right fix is, we should add a test that simply gets the start time several times for a process (eg., the tests process) to check it doesn't change. I assume that test would periodically fail with the current implementation. |
watfordgnf
force-pushed
the
issue-39434-macosx-process-starttime
branch
from
53d523e to
9e3b1c4
Compare
- Use the correct Assert.Equal too.
- SystemNative_GetAbsoluteTime - SystemNative_GetTimebaseInfo
|
Any more feedback @stephentoub |
| { | ||
| throw new Win32Exception(SR.RUsageFailure); | ||
| } | ||
| DateTime startTime = DateTime.UnixEpoch + TimeSpan.FromSeconds(info.Value.pbsd.pbi_start_tvsec + info.Value.pbsd.pbi_start_tvusec / MicrosecondsToSecondsFactor); |
There was a problem hiding this comment.
Should the division include a cast to double, e.g. pbsd.pbi_start_tvusec / (double)MicrosecondsToSecondsFactor?
There was a problem hiding this comment.
Related, have you done any manual testing of this to validate the results? It looks like our automated testing allows for a lot of wiggleroom on accuracy, around 6 seconds worth.
There was a problem hiding this comment.
For point (1) the factor is already a double. For point (2), I've reached out in the original issue for manual testing beyond CI.
I'll remove the wiggle room and see what shakes out.
There was a problem hiding this comment.
the factor is already a double
So it is. It's a bit strange that the two factors next to each other have different types.
There was a problem hiding this comment.
Disabling the wiggle room apparently fails on multiple platforms (other than OS X).
watfordgnf
force-pushed
the
issue-39434-macosx-process-starttime
branch
3 times, most recently
from
bd4b6d3 to
2cea534
Compare
|
@stephentoub does this address your feedback? If so this can merge. |
|
@stephentoub I think this has broken the build. We're getting: in #40063 |
|
I've created #40080 |
…net/corefx#39572) * Use pbi_start for StartTime on OS X fixes dotnet/corefx#39434 * Add test to ensure StartTime remains stable * Reduce delay in test based on feedback - Use the correct Assert.Equal too. * Ensure StartTime is in Local Time * Remove unnecessary test and reenable other test for OS X * Remove unused OS X interop methods - SystemNative_GetAbsoluteTime - SystemNative_GetTimebaseInfo * Remove cmake entries for unused HAVE_MACH_TIMEBASE_INFO * Address review comment regarding constants Commit migrated from dotnet/corefx@b368697
6 participants
This fixes #39434 by using a deterministic value for
Process.StartTimefrom thepbi_start_*values returned byGetProcessInfoByIdon Mac OS X.