Issue 27916: Use time.monotonic instead of time.time where appropriate (original) (raw)

Created on 2016-08-31 15:51 by nnnnnn, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
monotonic.patch nnnnnn,2016-08-31 15:51 review
Messages (7)
msg274036 - (view) Author: (nnnnnn) * 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: (nnnnnn) * 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: (nnnnnn) * 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
2022-04-11 14:58:35 admin set github: 72103
2019-03-11 14:50:08 cheryl.sabella set status: open -> closednosy: + cheryl.sabellamessages: + resolution: rejectedstage: resolved
2016-08-31 18:52:11 nnnnnn set messages: +
2016-08-31 18:31:52 gvanrossum set messages: +
2016-08-31 18:22:46 nnnnnn set messages: +
2016-08-31 18:01:51 gvanrossum set messages: +
2016-08-31 17:44:49 rhettinger set nosy: + rhettinger, gvanrossum, davinmessages: +
2016-08-31 15:51:16 nnnnnn create