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

add disk IoTime on freebsd and fix read & write time calculation #232

Merged
merged 1 commit into from
Jul 22, 2016
Merged

add disk IoTime on freebsd and fix read & write time calculation #232

merged 1 commit into from
Jul 22, 2016

Conversation

phemmer
Copy link

@phemmer phemmer commented Jul 21, 2016

This PR does 2 things.

  1. It adds the IoTime metric to FreeBSD
  2. It fixes the the disk time calculations.

In regards to number 2, the Compute() function was effectively doing nothing. It was converting BINTIME_SCALE to a uint64. But because the value is a float < 0, when converted to int64, it becomes 0. So the function effectively just returned the field b.Sec. This fixes the calculation by preserving the value as a float.
Also relatedly, the times were being returned in seconds, when the raw values offer much higher precision than that. Since the linux values are returned in milliseconds, I opted to keep the type (a uint64), but am now returning the values measured in milliseconds instead of seconds. (It would probably be a good idea to use a time.Duration instead of uint64)

@phemmer phemmer mentioned this pull request Jul 21, 2016
@shirou shirou merged commit ee66bc5 into shirou:master Jul 22, 2016
@shirou
Copy link
Owner

shirou commented Jul 22, 2016

Confirmed on my FreeBSD 10, great contribution. Thanks a lot!

(It would probably be a good idea to use a time.Duration instead of uint64)

Hmm, you're partly right. However, I want to avoid API change, and convert time.Duration from milliseconds is easy. so, I prefer it keeps.

@phemmer phemmer deleted the io_time_freebsd branch July 22, 2016 01:41
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