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

boot_time changing #1007

Closed
ericfrederich opened this issue Apr 6, 2017 · 7 comments
Closed

boot_time changing #1007

ericfrederich opened this issue Apr 6, 2017 · 7 comments

Comments

@ericfrederich
Copy link

ericfrederich commented Apr 6, 2017

Don't know if this is true for other platforms but on Windows I see different times.

data = {}
for _ in range(1000000):
  bt = psutil.boot_time()
  if bt not in data:
    data[bt] = 0
  data[bt] += 1

{1491482416.0: 870701, 1491482417.0: 129299}

I thought maybe this value should be cached. But that would only help to get the same answer within an existing process. In reality any process on that machine should return the same value right?
If it will only ever be off by 1 (because of rounding errors or whatever)... then maybe a better solution would be to always return an even number?

edit

Now that I think about it, to return a consistent number it would require you to know the two numbers it's alternating between. There doesn't seem to be a way to just call boot_time() once and perform some math on it and return a value. You need to know the two numbers it's alternating between.

@giampaolo
Copy link
Owner

What platform is this? Windows?

@ericfrederich
Copy link
Author

@giampaolo yes windows.... Sorry for the late reply.

giampaolo added a commit that referenced this issue May 8, 2017
…cceptable and return always the same value
@giampaolo
Copy link
Owner

giampaolo commented May 8, 2017

OK, so, I think I know what the problem is. At first I thought this was a C type precision issue but it's not.

GetSystemTimeAsFileTime and GetTickCount64 [1] are two synchronized clocks returning a value which changes every second, but given that they are two separate calls by the time the first function completes the second one may return a value which is +1 second higher (compared to the first function), so basically I think this is a race condition which we cannot avoid.

Your test script demonstrates this pretty clearly. The difference is always 1 second (never more) and it shows up only if you bash boot_time() hundreds of thousands of times (e.g. I didn't see it happen with 10.000 or 100.000 times).

As such the only thing we can do is store the value of the first call (in python) and return the cached value if the subsequent calls have a <= 1 sec fluctuation. I did this in 49ce1f6.

Since the difference is always max one second I don't think it's a big deal to return one value or the other, and we don't need to know the 2 numbers upfront.

A better solution would involve a timing function which has a precision > 1 second, so that we can then round() it, but honestly I don't know how to do that as that part of the code is pretty twisted already and I copied it from elsewhere.

[1]

psutil_boot_time(PyObject *self, PyObject *args) {

@giampaolo
Copy link
Owner

giampaolo commented May 8, 2017

I thought maybe this value should be cached. But that would only help to get the same answer within an existing process. In reality any process on that machine should return the same value right?

Oh, I missed this. Yes, you're right. My solution does not work across different processes. But as you said, we'd have to know the two numbers up front, which of course is not possible across separate processes. I guess we'll have to live with it, but in the end it really is a corner case which shows up over the course of 100k+ calls on average + multiple processes have to be involved.

giampaolo added a commit that referenced this issue May 8, 2017
giampaolo added a commit that referenced this issue May 8, 2017
@giampaolo
Copy link
Owner

Note to self: make sure that the time remains the same after hybernation.

@giampaolo giampaolo reopened this Jun 10, 2017
@igorpeshansky
Copy link

Thanks for fixing this issue. Would you be able to roll a release that includes the fix, so downstream projects can pick it up?

@giampaolo
Copy link
Owner

@igorpeshansky Sorry this issue was implemented long ago but I forgot to close it.

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

No branches or pull requests

3 participants