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 #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 #22043: "Use a monotonic clock to compute timeouts".
- PEP 410: "Use decimal.Decimal type for timestamps" (rejected)

Oh, I forgot to mention that the patch of the issue #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 #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

The effective* resolution

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.

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.

What problem does this solve?

>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"...

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.

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).

> 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."

> 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?

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.

> 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? :-)

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.

New changeset 2309597e7a00 by Victor Stinner in branch 'default':
Issue #22117: Add a new Python timestamp format _PyTime_t to pytime.h
https://hg.python.org/cpython/rev/2309597e7a00

New changeset f64d0b99d405 by Victor Stinner in branch 'default':
Issue #23451, #22117: Python 3.5 now requires Windows Vista or newer, so
https://hg.python.org/cpython/rev/f64d0b99d405

New changeset b0b4c4d365b1 by Victor Stinner in branch 'default':
Issue #22117: Fix test_gdb for the new time.sleep()
https://hg.python.org/cpython/rev/b0b4c4d365b1

New changeset 0378c10ba164 by Victor Stinner in branch 'default':
Issue #22117: Fix rounding in _PyTime_FromSecondsObject()
https://hg.python.org/cpython/rev/0378c10ba164

New changeset 45d9093d259d by Victor Stinner in branch 'default':
Issue #22117: time.monotonic() now uses the new _PyTime_t API
https://hg.python.org/cpython/rev/45d9093d259d

New changeset a88735cbeb50 by Victor Stinner in branch 'default':
Issue #22117: time.time() now uses the new _PyTime_t API
https://hg.python.org/cpython/rev/a88735cbeb50

New changeset abf38a17d3a8 by Victor Stinner in branch 'default':
Issue #22117: The gc module now uses _PyTime_t timestamp
https://hg.python.org/cpython/rev/abf38a17d3a8

New changeset a3c5e05d2cef by Victor Stinner in branch 'default':
Issue #22117: The signal modules uses the new _PyTime_t API
https://hg.python.org/cpython/rev/a3c5e05d2cef

New changeset f841d3bc30ee by Victor Stinner in branch 'default':
Issue #23618, #22117: refactor socketmodule.c
https://hg.python.org/cpython/rev/f841d3bc30ee

New changeset ae551abe398d by Victor Stinner in branch 'default':
Issue #22117: Write unit tests for _PyTime_AsTimeval()
https://hg.python.org/cpython/rev/ae551abe398d

New changeset c0c7d258c1ed by Victor Stinner in branch 'default':
Issue #22117: The socket module uses _PyTime_t timestamp for timeouts
https://hg.python.org/cpython/rev/c0c7d258c1ed

New changeset 7605d9d262ca by Victor Stinner in branch 'default':
Issue #22117: remove _PyTime_INTERVAL() macro
https://hg.python.org/cpython/rev/7605d9d262ca

New changeset d1ef5ff79125 by Victor Stinner in branch 'default':
Issue #22117: Fix ssl to use _PyTime_t API on sock_timeout
https://hg.python.org/cpython/rev/d1ef5ff79125

New changeset adbc9e6162fe by Victor Stinner in branch 'default':
Issue #22117: The thread module uses the new _PyTime_t timestamp API
https://hg.python.org/cpython/rev/adbc9e6162fe

New changeset 930be74bbae5 by Victor Stinner in branch 'default':
Issue #22117: Use the new _PyTime_t API in the select module
https://hg.python.org/cpython/rev/930be74bbae5

New changeset 5aa39b88bd55 by Victor Stinner in branch 'default':
Issue #22117: Use the _PyTime_t API for time.clock_settime()
https://hg.python.org/cpython/rev/5aa39b88bd55

New changeset 47123ac83733 by Victor Stinner in branch 'default':
Issue #22117: Add the new _PyTime_ROUND_FLOOR rounding method for the datetime
https://hg.python.org/cpython/rev/47123ac83733

New changeset 0850452048ec by Victor Stinner in branch 'default':
Issue #22117: Fix _PyTime_GetMonotonicClock() and
https://hg.python.org/cpython/rev/0850452048ec

New changeset e93eeadef0c3 by Victor Stinner in branch 'default':
Issue #22117: Use the _PyTime_t API in _datetime.datetime() constructor
https://hg.python.org/cpython/rev/e93eeadef0c3

New changeset b16fc95b66e4 by Victor Stinner in branch 'default':
Issue #22117: Cleanup pytime.c/.h
https://hg.python.org/cpython/rev/b16fc95b66e4

New changeset 626575d756da by Victor Stinner in branch 'default':
Issue #22117: Fix rounding and implement _PyTime_ROUND_FLOOR in:
https://hg.python.org/cpython/rev/626575d756da

New changeset c26b4b85dfc4 by Victor Stinner in branch 'default':
Issue #22117: Fix os.utime(), it now rounds the timestamp towards minus
https://hg.python.org/cpython/rev/c26b4b85dfc4

New changeset 6bae19f4e7b2 by Victor Stinner in branch 'default':
Issue #22117: Fix rounding of fromtimestamp() methods of datetime.datetime and
https://hg.python.org/cpython/rev/6bae19f4e7b2

New changeset d938533b43f7 by Victor Stinner in branch 'default':
Issue #22117: Fix usage of _PyTime_AsTimeval()
https://hg.python.org/cpython/rev/d938533b43f7

New changeset 49d3ff81f31f by Victor Stinner in branch 'default':
Issue #22117: Add assertions to _PyTime_AsTimeval() and _PyTime_AsTimespec() to
https://hg.python.org/cpython/rev/49d3ff81f31f

I rewrite the implementation of this issue 2 or 3 times. At the end, I decided to rewrite it again with a more incremental approach: add the new API, use the new API, and then drop slowly the old API.

I was used to rewrite the implementation multiple times to fix issues in the API and enhance the implementation. The new API now handles better rounding with more unit tests.

The datetime module only uses the new API for the datetime.datetime() constructor. It's not used for datetime.datetime.fromtimestamp(), because _PyTime_t is limited to a range of [-292 years; 292 years]. The datetime module supports a much large range: year in range [1; 9999]. So some parts of the old API will survive for the datetime module.

New changeset f6b41566ca28 by Victor Stinner in branch 'default':
Issue #22117: Add _PyTime_ROUND_CEILING rounding method for timestamps
https://hg.python.org/cpython/rev/f6b41566ca28

New changeset 0a8015a4ff97 by Victor Stinner in branch 'default':
Issue #22117: Replace usage of _PyTime_ROUND_UP with _PyTime_ROUND_CEILING
https://hg.python.org/cpython/rev/0a8015a4ff97

New changeset 350eb1ca561a by Victor Stinner in branch 'default':
Issue #22117: Remove _PyTime_ROUND_DOWN and _PyTime_ROUND_UP rounding methods
https://hg.python.org/cpython/rev/350eb1ca561a

Ok, I'm quite API with the new API and I have what I need to rework the select module for the PEP 475, so I close this issue.

May be rename _PyTime_AsTimeval_impl() to _PyTime_AsTimeval_noraise() and check a result to raise an exception in _PyTime_AsTimeval()?

>
> Serhiy Storchaka added the comment:
>
> May be rename _PyTime_AsTimeval_impl() to _PyTime_AsTimeval_noraise() and
> check a result to raise an exception in _PyTime_AsTimeval()?
>

Ah yes correct, can you modify directly the code please?

Hum, conversion from Python float to _PyTime_t is not rounded as expected on x86 Ubuntu Shared 3.x. Example:

http://buildbot.python.org/all/builders/x86%20Ubuntu%20Shared%203.x/builds/11426/steps/test/logs/stdio

======================================================================
FAIL: test_FromSecondsObject (test.test_time.TestPyTime_t) (obj=1e-06, round=<_PyTime.ROUND_FLOOR: 0>, timestamp=1000)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/test_time.py", line 779, in test_FromSecondsObject
    self.assertEqual(PyTime_FromSecondsObject(obj, rnd), ts)
AssertionError: 999 != 1000

======================================================================
FAIL: test_FromSecondsObject (test.test_time.TestPyTime_t) (obj=4194304.000000001, round=<_PyTime.ROUND_FLOOR: 0>, timestamp=4194304000000001)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/test_time.py", line 779, in test_FromSecondsObject
    self.assertEqual(PyTime_FromSecondsObject(obj, rnd), ts)
AssertionError: 4194304000000000 != 4194304000000001

New changeset e00b581a65ec by Victor Stinner in branch 'default':
Issue #22117: Try to fix rounding in conversion from Python double to _PyTime_t
https://hg.python.org/cpython/rev/e00b581a65ec

Oh, test_time now pass on "x86 Ubuntu Shared 3.x". It looks like the volatile keyword was enough to fix rounding issues, cool :-)

New changeset dbc92a254173 by Victor Stinner in branch 'default':
Issue #22117: Fix integer overflow check in socket_parse_timeout() on Windows
https://hg.python.org/cpython/rev/dbc92a254173

New changeset 65ac8e587bb0 by Victor Stinner in branch 'default':
Issue #22117, issue #23485: Fix _PyTime_AsMilliseconds() and
https://hg.python.org/cpython/rev/65ac8e587bb0

New changeset d976683671ba by Victor Stinner in branch 'default':
Issue #22117: Add a new _PyTime_FromSeconds() function
https://hg.python.org/cpython/rev/d976683671ba

New changeset af664f48b9b8 by Victor Stinner in branch 'default':
Issue #22117: Fix sock_call_ex() for non-blocking socket
https://hg.python.org/cpython/rev/af664f48b9b8

New changeset 3ab064e80a9be1e6e9c62437fffb92bde9c5e1fb by Victor Stinner in branch 'master':
bpo-23451: Update time.monotonic() documentation (GH-11190)
https://github.com/python/cpython/commit/3ab064e80a9be1e6e9c62437fffb92bde9c5e1fb

New changeset c367d52a74781b2c9ffd9e29722fbdfc0234408c by Miss Islington (bot) in branch '3.7':
bpo-23451: Update time.monotonic() documentation (GH-11190)
https://github.com/python/cpython/commit/c367d52a74781b2c9ffd9e29722fbdfc0234408c

Can somebody here explain the meaning of the comment in test_gdb.py

# Tested function must not be defined with METH_NOARGS or METH_O,
# otherwise call_function() doesn't call PyCFunction_Call()

This test is breaking with PEP 590, see https://github.com/python/cpython/pull/13185

Petr claims to have a fix, https://github.com/python/cpython/pull/13665

@Jeroen Demeyer: Both mentionned PRs are merged, so I understand that you found your answer. Commenting closed issues is not the most efficient way to get an answer ;-)

> Commenting closed issues is not the most efficient way to get an answer ;-)

Serious question: why is that? I got an email from bugs.python.org with your comment, so why should commenting on closed issues be any worse than commenting on open issues? Especially if the people on the issue are still active core developers.

Because this reduces the number of people which can notice your comment. Active core developers receive too much messages. Currently I have 473 unread messages from b.p.o (out of 22425) and 299 unread messages from GitHub (out of 20476). And this is only for open and recently closed issues (messages for closed issues are manually moved to other folders). There are also separate folders for other projects, mailing lists, etc. It is not surprising if I miss some messages.

Right, but my question was very specifically about a test added for this issue. So asking it here first made sense to me. If nobody would reply, I could still ask somewhere else.

In the end, Petr solved the problem anyway, so the question is irrelevant now.

> Right, but my question was very specifically about a test added for this issue.

This issue is about nanoseconds. I looked again and I found the commit  https://hg.python.org/cpython/rev/b0b4c4d365b1 related to this issue.

So to reply to your question, honestly, I don't recall why I had to replace sleep() with gmtime(): all I know is written in the comment. But Python internals changed *a lot* since 2015...

My comment is more general: closed issues are hidden from bugs.python.org home page, so only people in the nosy list get a notification. The discussion is "hidden". Since issue is 4 years old, some people in the nosy list no longer contribute to Python.

Moreover, the bug tracker is usually not a good place to ask a question. I prefer to only use it to discuss bugs.