-
-
Notifications
You must be signed in to change notification settings - Fork 886
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
#1061 Fix transferTime in WindowsDisks by using 1-%Idle #1063
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1063 +/- ##
===========================================
- Coverage 77.58% 77.5% -0.09%
Complexity 2 2
===========================================
Files 26 26
Lines 1249 1249
Branches 159 159
===========================================
- Hits 969 968 -1
- Misses 205 206 +1
Partials 75 75
Continue to review full report at Codecov.
|
It seems like it would work, except the default doesn't make sense (using 0 as default idle time means the disk is running 100%, it should be the opposite). But in my testing I can't see why it's wrong the other way. The idle and disk time always sum up to the same value, so the original method should work. Is it WMI or performance counters giving you the odd readings in the old code? |
How can I find out which method is used? What logging do I need to enable? |
Doesn't look like it's logged. You'd have to instrument the classes yourself. I'll try it tomorrow... manual WMI queries look sane, so it may be the performance counters don't read like I think they read |
I tried to do some further investigations but I failed. On the system where I see the transferTime == timeStamp in my app both Get-Counter and Get-CIMInstance return value of 0 for PercentDiskTime and 100 for PercentIdleTime in PowerShell ... how do you get the PERCENTDISKTIME? Is it maybe mapped incorrectly in JNA? I am quite lost but I am not that much into Windows :) |
But I think the reason to use 1 - % Idle Time was something like this: |
The "fallback" for percent disk time comes from WMI in the Win32_PerfRawData_PerfDisk_PhysicalDisk class. You can try this on your commandline: You'll notice they add up to the same number consistently. The primary choice of data, however, is from the performance counter Anyway, the 1-%Idle is probably a good thing to use and I'll accept this PR, but please look at your default as noted in my first comment. |
I just tested the disk time vs. idle time on my hard drive, over 20 seconds, got:
The numbers sum up to elapsed milliseconds (e.g., each row sum is about 1000 more than the previous). So this confirms subtracting idle time from elapsed time should give the correct result. |
…ld be set to 0 if no value is detected
Ok, I set default to stats.timeStamp. Then transferTime should be set to 0 if nothing is returned. |
Looks good, merging. If you want to submit the same PR against the 3.13.X branch I can update that one too. |
Hi,
first try at pull request. It compiles fine and uses 1 - %IdleTime to set the TransferTime.
Can you test this? I switched my system to Linux and don't have a test system at hand right now.
Thanks, Space