Skip to content
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

GetProcessTimes and GetProcessIoCounters #959

Merged
merged 2 commits into from
May 14, 2018

Conversation

dbwiddis
Copy link
Contributor

No description provided.

// lpExitTime is undefined for a running process, do not test
// Kernel and User time must be >= 0
assertTrue(lpKernelTime.toTime() >= 0);
assertTrue(lpUserTime.toTime() >= 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The asserts in lines 401 and 402 fail for me. The conversion in WinBase#FILETIME is implemented under the assumption, that a FILETIME is a timestamp. In this case this is wrong. the kernel and user times are relative times, that the process accumulated in user-/kernelmode. Two options: use FILEMTIME#toDWordLong or add a conversion to long to FILEMTIME.

The method binding for GetProcessTimes looks good, it is just the test, that das not work.

@dbwiddis
Copy link
Contributor Author

dbwiddis commented May 13, 2018

Whoops. My local version/test only used start time and got kernel/user from other sources. I added that at the last minute and forgot FILETIME uses the 1601 epoch.

DWORDLONG is the way to go here but since the value represents an unsigned long, a >= 0 test is silly. I’ll calculate an up time from the start time and calculate max ticks...

@matthiasblaesing
Copy link
Member

Thanks - this looks good. In theory the unittest could fail, as the kernel and user times are counted per thread according to MSDN. I looked at the numbers and at this time I think we should be save, if we see a problem at a later time we can still introduce a multiplier.

@matthiasblaesing matthiasblaesing merged commit 631c0e9 into java-native-access:master May 14, 2018
@dbwiddis
Copy link
Contributor Author

Ah, good point that you can get > 100% utilization on a multi-core processor. But the JUnit tests are single threaded, right?

@matthiasblaesing
Copy link
Member

The JUnit tests are single threaded, but there are background threads (GC). In practice I don't expect problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants