classification
Title: Use a monotonic clock to compute timeouts
Type: enhancement Stage:
Components: Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: belopolsky, haypo, neologix, python-dev
Priority: normal Keywords: patch

Created on 2014-07-23 02:10 by haypo, last changed 2014-09-17 22:17 by haypo. This issue is now closed.

Files
File name Uploaded Description Edit
pymonotonic.patch haypo, 2014-07-23 02:09 review
pytimespec.patch haypo, 2014-07-31 11:13 review
pymonotonic-2.patch haypo, 2014-07-31 11:13
pymonotonic-3.patch haypo, 2014-08-29 15:30 review
pymonotonic-4.patch haypo, 2014-08-31 13:02 review
Messages (18)
msg223715 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-07-23 02:09
Currently, socket methods (ex: accept, recv, recvfrom, recvmsg, send, sendto, sendmsg), threading.Lock.acquire() and threading.RLock.acquire() use the system clock to compute their timeout. It's fine for the first call. But if the call is interrupted and the timeout need to be recomputed, it can wait too short or too long. Timeouts must use a monotonic clock, not the system clock. See the PEP 418 for more information.

Python modules were already patched to use the time.monotonic() function implemented in Python 3.3.

Attached patch fixes also functions which still use the system clock to compute timeouts.

A major change of the patch is that a monotonic clock is now require to use Python 3.5. Last time I checked, there was only one OS without monotonic clock: GNU Hurd. Hurd maintainers can patch Python 3.5 to fallback on the system clock until Hurd provides a monotonic clock.

Another important change is that Python now depends on the librt on Solaris and on Linux with glibc older than 2.17 (clock_gettime is now available directly in the libc since glibc 2.17).
msg223716 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-07-23 02:11
On FreeBSD and OpenBSD, clock_gettime() is directly available in the libc.
msg224365 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-07-31 00:06
Status of CLOCK_MONOTONIC in Hurd:
https://github.com/ArneBab/hurd-web/blob/master/open_issues/clock_gettime.mdwn
msg224390 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-07-31 11:13
pymonotonic.patch is large and difficult to review. I prefer to split it into two parts:

- pytimespec.patch: Change pytime.h to use _PyTimeSpec structure (nanosecond resolution) instead of _PyTime_timeval structure (microsecond resolution)

- pymonotonic-2.patch: Add _PyTimeSpec_monotonic() and _PyTimeSpec_monotonic_info()
msg224392 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-07-31 11:24
pytimespec.patch removes *private* functions which were exposed in the stable ABI:

- _PyTime_gettimeofday()
- _PyTime_gettimeofday_info()

It also removes private macros:

- _PyTime_ADD_SECONDS()
- _PyTime_INTERVAL()

In pymonotonic.patch, I reused the same name but I didn't notice that they were part of the stable ABI. In pymonotonic-2.patch, I changed names so the dynamic loader will not load a module and the compiler will complain that symbols are missing.

Replaced/renamed functions:

- _PyTime_gettimeofday() => _PyTimeSpec_get_time()
- _PyTime_gettimeofday_info() => _PyTimeSpec_get_time_info()
- _PyTime_ADD_SECONDS() => _PyTimeSpec_add_sec()
- _PyTime_INTERVAL() => _PyTimeSpec_interval_sec()

I aslo added new functions:

- _PyTimeSpec_add_us()
- _PyTimeSpec_interval_us()

On overflow, _PyTimeSpec_add_sec() and _PyTimeSpec_add_us() sets the timestamp to the maximum value instead of returning an undefined value. I prefer functions over macros because I want to return an error on overflow (even if it's not used).

_PyTimeSpec_interval_us() also returns the maximum value in case of overflow.

_PyTimeSpec_interval_sec() should maybe take a mandatory round parameter instead of always rounding up.

I added _PyTime_unit_t for _threadmodule.c because acquire_timed() has a PY_TIMEOUT_T type which can be a long long. Maybe acquire_timed() can return an error if the timeout is too large, or loop? I would prefer to avoid this custom _PyTime_unit_t type.

I should maybe add unit tests for _PyTimeSpec_add_*() and _PyTimeSpec_interval_*() functions, as I did for _PyLong_FromTime_t(), _PyTime_ObjectToTimeval(), etc.
msg224393 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-07-31 11:25
Oh, pymonotonic-2.patch didn't get its "review" link because it depends on pytimespec.patch which is not merged yet.
msg224456 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-07-31 23:39
pytimespec.patch become too large, I splitted this patch into a new issue: issue #22117. If this issue is rejected, I will rewrite  pymonotonic-2.patch to use the _PyTime_timeval structure.
msg225969 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-08-27 10:53
To have an even smaller patch, I created the issue #22287 just to add the dependency to the librt module in pytime.c.
msg226064 - (view) Author: Roundup Robot (python-dev) Date: 2014-08-29 14:48
New changeset 668e0bf30042 by Victor Stinner in branch 'default':
Issue #22043: _PyTime_Init() now checks if the system clock works.
http://hg.python.org/cpython/rev/668e0bf30042
msg226065 - (view) Author: Roundup Robot (python-dev) Date: 2014-08-29 14:58
New changeset 76bc15c918b1 by Victor Stinner in branch 'default':
Issue #22043: Simplify time.perf_counter() on Windows
http://hg.python.org/cpython/rev/76bc15c918b1
msg226066 - (view) Author: Roundup Robot (python-dev) Date: 2014-08-29 15:01
New changeset ab81b4cdc33c by Victor Stinner in branch 'default':
Issue #22043: Oops, fix perf_counter() on UNIX if no monotonic clock is
http://hg.python.org/cpython/rev/ab81b4cdc33c
msg226067 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-08-29 15:30
Ok, I prepared Python for monotonic clock, I attached an updated patch. It is now much simpler.

pymonotonic-3.patch:

* time.monotonic() is now always available

* _PyTime_Init() ensures that the operating system provides a monotonic clock and that the clock works

* Python 3.5 now requires a monotonic clock. All operating systems supported by Python 3.5 provides a monotonic clock. GNU Hurd doesn't, but it is not supported.

* drop try/except ImportError around "from time import monotonic"

* use a monotonic clock in _thread, gc and socket modules to compute elapsed time and timeouts
msg226175 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-08-31 13:03
pymonotonic-4.patch: Updated patch (version 4) to address Antoine Pitrou's comments on Rietveld.
msg226283 - (view) Author: Roundup Robot (python-dev) Date: 2014-09-02 21:01
New changeset 330bd57685fc by Victor Stinner in branch 'default':
Issue #22043: Fix _PyTime_gettimeofday() if HAVE_GETTIMEOFDAY
http://hg.python.org/cpython/rev/330bd57685fc
msg226284 - (view) Author: Roundup Robot (python-dev) Date: 2014-09-02 21:26
New changeset b12857782041 by Victor Stinner in branch 'default':
Issue #22043: time.monotonic() is now always available
http://hg.python.org/cpython/rev/b12857782041
msg226301 - (view) Author: Roundup Robot (python-dev) Date: 2014-09-03 07:44
New changeset 9deef14393d5 by Victor Stinner in branch 'default':
Issue #22043: Fix pymonotonic(), use tv_usec=-1 as a marker to skip
http://hg.python.org/cpython/rev/9deef14393d5
msg226302 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-09-03 07:58
Thanks for the review Antoine. I pushed the new version pymonotonic-4.patch with a minor change: in debug mode, pymonotonic() also ensures that the clock never goes backward!
msg227016 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-09-17 22:17
Buildbots are happy, I close the issue.
History
Date User Action Args
2014-09-17 22:17:19hayposetstatus: open -> closed
resolution: fixed
messages: + msg227016
2014-09-03 07:59:00hayposetmessages: + msg226302
2014-09-03 07:44:06python-devsetmessages: + msg226301
2014-09-02 21:26:16python-devsetmessages: + msg226284
2014-09-02 21:01:56python-devsetmessages: + msg226283
2014-08-31 13:03:20hayposetmessages: + msg226175
2014-08-31 13:02:49hayposetfiles: + pymonotonic-4.patch
2014-08-29 15:30:17hayposetfiles: + pymonotonic-3.patch

messages: + msg226067
2014-08-29 15:01:03python-devsetmessages: + msg226066
2014-08-29 14:58:35python-devsetmessages: + msg226065
2014-08-29 14:48:13python-devsetnosy: + python-dev
messages: + msg226064
2014-08-27 10:53:41hayposetmessages: + msg225969
2014-07-31 23:39:27hayposetmessages: + msg224456
2014-07-31 11:25:04hayposetmessages: + msg224393
2014-07-31 11:24:28hayposetmessages: + msg224392
2014-07-31 11:13:33hayposetfiles: + pymonotonic-2.patch
2014-07-31 11:13:17hayposetfiles: + pytimespec.patch

messages: + msg224390
2014-07-31 00:13:32pitrousetnosy: + neologix
2014-07-31 00:11:39belopolskysetnosy: + belopolsky
2014-07-31 00:06:48hayposetmessages: + msg224365
2014-07-23 02:11:24hayposetmessages: + msg223716
2014-07-23 02:10:01haypocreate