Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fix System.Diagnostics.Process.[x]ProcessorTime on macOS#37637

Merged
danmoseley merged 8 commits intodotnet:masterfrom
gregkalapos:ProcessXProcessorTimeFix
May 15, 2019
Merged

Fix System.Diagnostics.Process.[x]ProcessorTime on macOS#37637
danmoseley merged 8 commits intodotnet:masterfrom
gregkalapos:ProcessXProcessorTimeFix

Conversation

@gregkalapos
Copy link
Copy Markdown
Contributor

@gregkalapos gregkalapos commented May 14, 2019

Fixes #37614

Affects on macOS:

  • Process.UserProcessorTime
  • Process.PrivilegedProcessorTime
  • Process.TotalProcessorTime

Discussed with @stephentoub in #37614, this PR implements the proposed fix.

Short summary (more in #37614):
The rusage_info_v3 mac API uses nanoseconds unit, which was passed without any conversion to TimeSpan’s Int64-based constructor, which expects a value with a 100-nanosecond unit. Therfore values reported on macOS were 100x of the correct value.

Fix: we simply convert the rusage_info_v3 nanosec values to 100-nanosec unit.

Also added a test, which fails with the original code on my machine and passes with the code in this PR. Since the TestProcessorTime test is disabled for UWP (https://github.com/dotnet/corefx/issues/31908) I suspect we will have similar problems with the new test. Let's see...

@gregkalapos
Copy link
Copy Markdown
Contributor Author

Windows UWP_CoreCLR_x64_Debug failed

2019-05-14T01:49:45.6078346Z C:\Users\VssAdministrator\.nuget\packages\microsoft.dotnet.helix.sdk\2.0.0-beta.19257.7\tools\Microsoft.DotNet.Helix.Sdk.MultiQueue.targets(67,5): error : Work item System.Diagnostics.Process.Tests in job f9494a0a-8179-40f0-ac7c-f7301c67a32e has failed. [D:\a\1\s\eng\sendtohelix.proj]

Looks very similar to #31908 - cc @ViktorHofer.

Added [ActiveIssue(31908, TargetFrameworkMonikers.Uap)] to the new test.

Comment thread src/System.Diagnostics.Process/src/System/Diagnostics/Process.OSX.cs Outdated
Comment thread src/System.Diagnostics.Process/tests/ProcessTests.cs Outdated
Comment thread src/System.Diagnostics.Process/tests/ProcessTests.cs
Comment thread src/System.Diagnostics.Process/src/System/Diagnostics/Process.OSX.cs Outdated
@danmoseley
Copy link
Copy Markdown
Member

I'll merge since you can fix in that other PR.

@danmoseley danmoseley merged commit a28176b into dotnet:master May 15, 2019
@danmoseley
Copy link
Copy Markdown
Member

Thanks @gregkalapos ! After fixing the typo, we would welcome other contributions.
We have many issues up for grabs
https://github.com/dotnet/corefx/issues?q=is%3Aopen+is%3Aissue+label%3Aup-for-grabs
we are most interested in those marked 3.0 milestone
https://github.com/dotnet/corefx/issues?q=is%3Aopen+is%3Aissue+label%3Aup-for-grabs+milestone%3A3.0

Any interest?

@karelz karelz added this to the 3.0 milestone May 22, 2019
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…fx#37637)

* Fix System.Diagnostics.Process xProcessorTime props and add test

The ri_user_time unit is nanoseconds, but the TimeSpan .ctor expects values with a 100-nanosecond unit.

* Update src/System.Diagnostics.Process/tests/ProcessTests.cs

* Update src/System.Diagnostics.Process/tests/ProcessTests.cs

* Update src/System.Diagnostics.Process/tests/ProcessTests.cs

* Update src/System.Diagnostics.Process/tests/ProcessTests.cs

* Add ActiveIssue to TotalProcessorTime_PerformLoop_TotalProcessorTimeValid

* Address PR feedback

* Fix typo - NanoSeconds -> Nanoseconds


Commit migrated from dotnet/corefx@a28176b
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Process.GetCurrentProcess().TotalProcessorTime returns strange values on macOS

4 participants