classification
Title: Use time.monotonic instead of time.time where appropriate
Type: enhancement Stage: resolved
Components: Versions:
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: cheryl.sabella, davin, gvanrossum, rhettinger, scop
Priority: normal Keywords: patch

Created on 2016-08-31 15:51 by scop, last changed 2019-03-11 14:50 by cheryl.sabella. This issue is now closed.

Files
File name Uploaded Description Edit
monotonic.patch scop, 2016-08-31 15:51 review
Messages (7)
msg274036 - (view) Author: Ville Skyttä (scop) * Date: 2016-08-31 15:51
For better immunity against system clock changes.
msg274047 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-08-31 17:44
Some of these look gratuitous to me, possibly a wholesale search and replace without detailed consideration of each change.  For example, the quick informational timing in random.py does not have any downside for using the regular time.time().  The changes to asyncio and multiprocessing may be warranted.

Davin, would you look at the multiprocessing portion of the patch?

Guido, can you nosy whoever the relevant maintainer is for the asyncio niggling details?
msg274050 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-08-31 18:01
asyncio already uses monotonic. The only place patched is a test
utility and I don't care either way. But I do think this is not a good
patch to do in general.
msg274057 - (view) Author: Ville Skyttä (scop) * Date: 2016-08-31 18:22
It is not a wholesale search and replace, I have checked each change. There are a lot of places in the tree where monotonic is not appropriate, and those should not be included in the patch. But there is always the possibility that I've missed something, and it's good that there are reviewers.

Granted, some of the changes in this patch might not make a big difference, but they should not hurt either. To me, using monotonic is the right thing to use when dealing with time differences/delays, unless one needs an actual predictable time value anchored at the epoch at the same time for some other purposes. I don't of course insist on that point of view, and would be interested to hear other opinions and if someone can point out flaws/dangers in mine.
msg274058 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-08-31 18:31
I mostly adhere to the rule of thumb "if it ain't broke, don't fix
it". So I would only endorse such changes if they address actual pain,
rather than possible pain.

Note that that rule of thumb was born out of worry about another
possible pain: the observation that some fraction of well-intentioned
fixes actually break something unanticipated. An (imagined) example in
this case: if someone mocks time.time() in a unit test, your fix
*might* break their test.
msg274059 - (view) Author: Ville Skyttä (scop) * Date: 2016-08-31 18:52
Thanks for the explanation, understood. However, I don't think it would be terribly difficult to demonstrate that these changes do address actual pains(*) in the situations they're supposed to address them. And the pains in question might in real world occur quite rarely, unexpectedly, and take some time/cost to debug and reproduce, which is why it would be good to be able to preemptively address them.

(*) For some values of "pain" -- many of the changes here are just for outputting nice-to-know timing information.
msg337672 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2019-03-11 14:50
multiprocessing was changed to use time.monotonic in issue 34054.  Based on the other feedback on this thread, I'm going to close this.
History
Date User Action Args
2019-03-11 14:50:08cheryl.sabellasetstatus: open -> closed

nosy: + cheryl.sabella
messages: + msg337672

resolution: rejected
stage: resolved
2016-08-31 18:52:11scopsetmessages: + msg274059
2016-08-31 18:31:52gvanrossumsetmessages: + msg274058
2016-08-31 18:22:46scopsetmessages: + msg274057
2016-08-31 18:01:51gvanrossumsetmessages: + msg274050
2016-08-31 17:44:49rhettingersetnosy: + rhettinger, gvanrossum, davin
messages: + msg274047
2016-08-31 15:51:16scopcreate