Rewrite pytime.h to work on nanoseconds

[vstinner]

BPO	22117
Nosy	@tim-one, @loewis, @abalkin, @pitrou, @vstinner, @serhiy-storchaka, @jdemeyer, @koobs, @miss-islington
PRs	convert from long long to PyLong losslessly #1106 bpo-23451: Update time.monotonic() documentation #11190 [3.7] bpo-23451: Update time.monotonic() documentation (GH-11190) #11191 gh-79888: support __index__ and __float__ in time functions #11636
Files	timespec.patch timespec-2.patch timespec-3.patch nanosec-wip.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2015-03-30.09:52:22.799>
created_at = <Date 2014-07-31.23:08:13.499>
labels = ['type-feature']
title = 'Rewrite pytime.h to work on nanoseconds'
updated_at = <Date 2019-07-04.11:05:28.158>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2019-07-04.11:05:28.158>
actor = 'jdemeyer'
assignee = 'none'
closed = True
closed_date = <Date 2015-03-30.09:52:22.799>
closer = 'vstinner'
components = []
creation = <Date 2014-07-31.23:08:13.499>
creator = 'vstinner'
dependencies = []
files = ['36186', '36197', '36200', '36538']
hgrepos = []
issue_num = 22117
keywords = ['patch']
message_count = 49.0
messages = ['224452', '224460', '224461', '224527', '224530', '224560', '224570', '224571', '225861', '225906', '225918', '225919', '225920', '226384', '239395', '239402', '239405', '239415', '239440', '239443', '239448', '239451', '239452', '239453', '239528', '239530', '239531', '239542', '239552', '239558', '239559', '239566', '239568', '239579', '239580', '239590', '239718', '239823', '239969', '240181', '331983', '331990', '343914', '343915', '344711', '344712', '344717', '344718', '344737']
nosy_count = 10.0
nosy_names = ['tim.peters', 'loewis', 'belopolsky', 'pitrou', 'vstinner', 'python-dev', 'serhiy.storchaka', 'jdemeyer', 'koobs', 'miss-islington']
pr_nums = ['1106', '11190', '11191', '11636']
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue22117'
versions = ['Python 3.5']

[vstinner]

To prepare Python to add support of monotonic clock in pytime.h, I propose to rewrite pytime.h to use a new _PyTimeSpec structure which has a resolution of 1 nanosecond on only work on integers.

Currently, pytime.h uses a _PyTime_timeval structure which has a solution of 1 microsecond and works internally on floating point numbers. Computing the difference between two timestamps may loose precision.

The tv_nsec field of _PyTimeSpec must be in the range [0; 999999999], even for negative numbers. For example, -1 ns is stored as (-1, 999999999). This property makes the code more complex, especially to round correctly.

The new API is based on the idea that all timestamps must be stored as _PyTimeSpec.

Convert a value into a _PyTimeSpec:

• _PyTimeSpec_from_double(): C double
• _PyTimeSpec_from_object(): Python int or float object
• you can also fill directly the tv_sec and tv_nsec fields

Convert a _PyTimeSpec timestamp into a value:

• _PyTimeSpec_to_time_t(): C time_t
• _PyTimeSpec_to_timeval(): C timeval (tv_sec, tv_usec), but ot directly the structure because the exact structure is different depending on the OS and OS version
• _PyTimeSpec_ms(): C long, number of milliseconds

Operations on _PyTimeSpec:

• _PyTimeSpec_add(): a+b
• _PyTimeSpec_sub(): a-b

I removed high-level functions like _PyTime_ObjectToTimeval(): you should now combine _PyTimeSpec_from_object() with _PyTimeSpec_to_timeval(). I did this to not multiply the number of functions, because I want to test all functions in unit tests and have a short API.

I tried to enhance detecton of overflow. I didn't review carefuly my patch yet. I'm not sure that all calls check for error and handle exceptions correctly. Only the following functions raise an exception on error:

• _PyTimeSpec_from_object()
• _PyTimeSpec_to_time_t()
• _PyTimeSpec_to_timeval()

Other functions sets the minimum/maximum value in case of underflow/overflow. So even if you don't check for errors, the behaviour on overflow should be acceptable.

The new _PyTimeSpec_Init() function checks that the system clock works, so _PyTimeSpec_get_time() and _PyTimeSpec_get_time_info() don't need to check for error. In fact, these functions call Py_FatalError() on error, but it should never happen. This idea was proposed in the issue bpo-22043 to check if the monotonic clock is working. Maybe we can avoid checking the system clock in _PyTimeSpec_Init() and only check the monotonic clock. I hope that all platforms have a working system clock! The oppoosite would be a huge issue.

The patch removes function which are exposed in the stable ABI. Since the functions are private, name starting with ("_Py"), I consider that it's ok to remove them. The functions are replaced with new functions which a have a new name. I excluded pytime.h from the stable ABI. I don't know why private functions were part of the stable ABI :-/

The patch comes with unit tests for each function.

I didn't implement handling of overflow and underflow in _PyTimeSpec_add() and _PyTimeSpec_sub() yet. But I implemented detection for overflow for the simple case.

See also:

• Issue bpo-22043: "Use a monotonic clock to compute timeouts".
• PEP-410: "Use decimal.Decimal type for timestamps" (rejected)

[vstinner]

Oh, I forgot to mention that the patch of the issue bpo-22043 also changes _PyTimeSpec_get_time() to use clock_gettime(CLOCK_REALTIME) which has a resolution of 1 nanosecond.

_PyTimeSpec_get_time() already uses GetSystemTimeAsFileTime() which has a resolution of 100 nanosecond. See also the issue bpo-19007 "precise time.time() under Windows 8: use GetSystemTimePreciseAsFileTime".

I'm talking about the resolution of the C structure. The effive resolution can be much worse than that. For example, the resolution measured in Python of clock_gettime(CLOCK_REALTIME) is closer to 160 nanoseconds (on my laptop) than 1 nanoescond. See the "Python Resolution" column the second table of:
http://legacy.python.org/dev/peps/pep-0418/#system-time

[vstinner]

The effective* resolution

[vstinner]

Here is a more complete patch (version 3). It uses _PyTimeSpec in more functions.

I tested the patch on Linux, Windows, FreeBSD, OpenBSD. I was surprised to find bugs. For example, Windows has a time_t larger than long, whereas OpenBSD 5.4 (on 64 bit) has a time_t smaller than long.

[vstinner]

timespec-3.patch is quite large:

Include/pyport.h | 14 +
Include/pytime.h | 159 +++++++----
Lib/test/test_time.py | 340 +++++++++++++++++++------
Modules/_datetimemodule.c | 40 ++
Modules/_ssl.c | 31 +-
Modules/_testcapimodule.c | 238 ++++++++++++++++-
Modules/_threadmodule.c | 196 +++++++-------
Modules/gcmodule.c | 15 -
Modules/posixmodule.c | 24 -
Modules/selectmodule.c | 36 --
Modules/signalmodule.c | 12
Modules/socketmodule.c | 206 +++++++++------
Modules/socketmodule.h | 6
Modules/timemodule.c | 22 -
Python/pythonrun.c | 3
Python/pytime.c | 616 +++++++++++++++++++++++++++++++++-------------
16 files changed, 1371 insertions(+), 587 deletions(-)

Tell me if you prefer to review shorter patches. I can try to only add new functions, then use new functions, and finally remove new functions.

I wanted to keep changes in a single patch at least one to show how the new API is used.

[loewis]

What problem does this solve?

[vstinner]

Tell me if you prefer to review shorter patches. I can try to only add new functions, then use new functions, and finally remove new functions.

Oh, i should read what i wrote before pushing the submit button. The last
part is"remove the old functions"...

[vstinner]

Le samedi 2 août 2014, Martin v. Löwis <report@bugs.python.org> a écrit :

What problem does this solve?

My patch detects overflows which are not detected yet. Currently i guess
that the behaviour on overflow is undefined. I should test each function.

I also want a nanosecond resolution. It's the same motivation than the PEP-410, except that the patch doesn't touch the Python API, I only care of the
C code which has fewer constraints (we don't need a "full" API). I expect
that C code is more concerned by the resolution because C code is faster.
You need to recompute timeout on EINTR (see the PEP-475 which is still a
draft). I don't want to loose precision and I want to round correctly.

My main usecase is to compute a timeout from two timestamps of a monotonic
clock.

IMO it's better to use PyTimeSpec structure everywhere to reuse the code
which is now well tested (by unit tests) and make the code more consistent.
For example, the datetime module rounds currently using the "down" method,
whereas it needs to round "floor" in fact. I saw this issue when I changed
the code to use PyTimeSpec in all functions.

I agree that my patch is large, especially the new code in pytime.c. I
would like to hide the complexity in functions, the API should be
simple. Do you think that changes in modules like time, socket or select
make the code more complex? I don't know if it's worth it.

[vstinner]

Instead of a complex structure, we can use a 64-bit signed integer to store a number of nanoseconds. For a UNIX epoch, nanoseconds since January 1st 1970, the min/max are:
1677-09-21 00:12:43.145224
2262-04-11 23:47:16.854776

The Linux kernel is going to use 64-bit integer even on 32-bit CPU to store timestamps, to simplify the code (to avoid the structure).

[vstinner]

The Linux kernel is going to use 64-bit integer even on 32-bit CPU to store timestamps, to simplify the code (to avoid the structure).

Read this article: http://lwn.net/Articles/607741/

"One of the first changes merged for 3.17 is to simply get rid of the non-scalar form of ktime_t and force all architectures to use the 64-bit nanosecond count representation. This change may slow things down on 32-bit systems; in particular, conversions from other time formats may be significantly slower. But, as noted in the changelog, the ARM and x86 architectures were already using the scalar format anyway, so they will not get any slower."

[pitrou]

Instead of a complex structure, we can use a 64-bit signed integer to store a number of nanoseconds.

Do we have 64-bit integers on all architectures?

[loewis]

Am 26.08.14 15:32, schrieb Antoine Pitrou:

Antoine Pitrou added the comment:

> Instead of a complex structure, we can use a 64-bit signed integer to store a number of nanoseconds.

Do we have 64-bit integers on all architectures?

On all "supported" architectures, yes. gcc supports long long
everywhere, using a library if hardware support is not feasible.
Visual C++ supports __int64 on all targets. Other compilers typically
strive for compatibility with either of these two.

[vstinner]

Do we have 64-bit integers on all architectures?

That's a good question! Visual Studio provides __int64. GCC provides "long long" (64 bit on 32 bit platform). I guess that ICC also supports int64_t.

It would be a shame to not support 64-bit integers in 2014, no? :-)

[vstinner]

And now, something completly different.

nanosec-wip.patch is a work-in-progress patch to use a unified (64 bits) integer type to store a timestamp: _PyTime_t. The type is written to be opaque, the unit is undefined, you must use functions to convert from and to this type. Well, in fact it's just a number of nanoseconds.

The patch is large:

Include/pytime.h | 125 ++++++-----
Modules/_datetimemodule.c | 43 ++--
Modules/_testcapimodule.c | 25 +-
Modules/_threadmodule.c | 156 +++++++-------
Modules/gcmodule.c | 12 -
Modules/posixmodule.c | 29 +-
Modules/selectmodule.c | 33 ---
Modules/signalmodule.c | 20 -
Modules/socketmodule.c | 151 ++++++++------
Modules/socketmodule.h | 2
Modules/timemodule.c | 40 ++-
Python/pytime.c | 480 +++++++++++++++++++++++++++++-----------------
12 files changed, 645 insertions(+), 471 deletions(-)

Conversion functions require a rounding mode.

I didn't update all tests yet. For example, test_datetime crash, test_time and test_threading fail. There are also 3 remaining FIXME for myself in pytime.c.