-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Introducing Environment.CpuUsage #105152
Introducing Environment.CpuUsage #105152
Conversation
Note regarding the
|
1 similar comment
Note regarding the
|
Tagging subscribers to this area: @dotnet/area-system-runtime |
src/libraries/System.Private.CoreLib/src/System/Environment.SunOS.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Environment.Linux.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Environment.Linux.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Environment.Browser.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Environment.iOS.cs
Outdated
Show resolved
Hide resolved
02e0c76
to
14404e6
Compare
...braries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RuntimeMetrics.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs
Outdated
Show resolved
Hide resolved
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.
LGTM
@jkotas @stephentoub now all feedback is addressed, and we have a clean CI run (I am just rerunning now the unrelated failed tests, but everything related to my change is passing). Thanks again for all feedback. I assume we are good to go now. |
...braries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RuntimeMetrics.cs
Outdated
Show resolved
Hide resolved
...braries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RuntimeMetrics.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Environment.UnixOrBrowser.cs
Outdated
Show resolved
Hide resolved
Assert.True(usage.TotalTime - totalTime < delta); | ||
|
||
Thread.Sleep(100); | ||
} |
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.
Would any of these tests fail if we had a bug where, for example, we reported 5ms instead as 5us or vice versa?
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.
I am not sure we can be that accurate without writing the tests calling the underlying OS. I tried to look at the Process tests and couldn't find a test doing that either.
Fixes #104844.