psutil.disk_io_counters returns microseconds instead of seconds

[FilmCoder]

Bug occured on these systems:

Windows 10: Python 2.7.12 (v2.7.12:d33e0cf91556, Jun 27 2016, 15:19:22) [MSC v.1500 32 bit (Intel)] on win32
Windows 7: Python 2.7.10 (default, May 23 2015, 09:44:00) [MSC v.1500 64 bit (AMD64)] on win32

Bug did NOT occur with this Arch Linux system:
Python 2.7.13 (default, Dec 21 2016, 07:16:46)

Note: Bug tested to happen on both Windows 10 and Windows 7.

Example output on Windows:

>>> psutil.disk_io_counters()
sdiskio(read_count=4196177, write_count=6160257, read_bytes=87497976832L, write_bytes=125426715648L, read_time=1987935564L, write_time=1489314922L)

query disk_io_counters() at 1 second intervals consistently results in the write_time being more than 5000 in difference. However, if the units were in milliseconds you can't spend more than 1000 milliseconds writing to disk within a second.

It seems that the units are microseconds. I spun up perfmon.exe and its numbers match to what i'm getting with python when I convert from microseconds to milliseconds.

[giampaolo]

To be clear: if you divide psutil's time-related results by 1000 you get the same numbers as perfmon.exe?

[giampaolo]

Up?

[FilmCoder]

That is exactly correct.

[mechalynx]

From my own testing (gist) it seems disk_io_counters returns read_time and write_time in tens of microseconds, not actual microseconds. To match perfmon's values (LogicalDisk\Avg. Disk sec/read for example, check the Last indicator) you need to divide by 1e5.

I checked the implementation in psutil and I found this to be the commit that implemented disk_io_counters: 199d109

You can see that it multiplies the value by 10 before dividing by 1000. The implementation of that particular value fetch is still the same. The commit doesn't have any message to explain why this is done, but I'm guessing it's to make the value a proper integer instead of a float.

I tested it locally (Win7 pro x64) and neither read_time nor write_time ever use their last digit, it's always zero. So that multiplication by 10 might have been necessary in the past, but it apparently serves no purpose anymore, at least on Win7.

I would recommend either changing the documentation to indicate that on Windows the returned values are in tens of microseconds (so that the code doesn't need to be modified if it works on other systems) or removing the x10 and stating that it returns microseconds on Windows. I'm assuming milliseconds are returned on other systems so this has to be documented for Windows explicitly.

[giampaolo]

Sorry for being late. The values are supposed to be in milliseconds on all platforms including Windows. I don't have time to look into this right now but it seems some of you guys have figured out the math already. Can somebody provide a PR or tell me the math operation to use instead of the current (x * 10) / 1000?

[mechalynx]

The same operation won't work on all platforms because Windows returns different values than the Unixes do. I don't think a change needs to be made - just updating the documentation should be enough. However, if code is to be adjusted so that it returns milliseconds on Windows, just detect if the OS is Windows and divide by 1e6 instead of 1e3, without the multiplication by 10 first.

Is there any reason not to just return microseconds on all platforms? The adjustment for Windows would still be needed (division by 1000) but I think it's better to just return the most precise value available and leave it to the user to adjust it to the order of magnitude they wish. As long as the documentation is accurate, this shouldn't be a problem although psutil is widely used enough that changing the order of magnitude abruptly might not be a good idea.

There can be a compromise: return both values separately (raw and milliseconds) and just adjust the value differently on Windows to get the ms.

[giampaolo]

Is there any reason not to just return microseconds on all platforms?

Portability. It's easier to declare that milliseconds are returned on all platforms so that user won't have to do:

if WINDOWS:
     ...
else:
     ...

I'll just divide by 1e6 then, thanks.

[mechalynx]

Sorry, I was wrong:

((1*10)/1000)/1e5 = 1e-7

so the equivalent to what I've been doing to get milliseconds would be to divide by 1e7, not 1e6. I tested this using the latest build from AppVeyor for Windows and confirmed it.

Portability. It's easier to declare that milliseconds are returned on all platforms

In that sense, portability is a matter of consistency between platforms, not what unit you use. So milliseconds or microseconds would work just as well. I don't mind either way, just clarifying.