Issue 9079: Make gettimeofday available in time module (original) (raw)
Created on 2010-06-25 00:52 by belopolsky, last changed 2022-04-11 14:57 by admin. This issue is now closed.
Messages (43)
Author: Alexander Belopolsky (belopolsky) * 
Date: 2010-06-25 00:52
Attached patch moves cross-platform logic of obtaining current time to _time.c which is shared between time and datetime modules. This simplifies both modules and reduces the datetime module dependency on the time module.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-06-25 02:35
The new patch, .diff exposes gettimeofday as time.gettimeofday() returning (sec, usec) pair.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-06-25 14:09
Mark,
I am reassigning this to you for commit review. I am changing the title to reflect the visible part of the change. The datetime module gains direct access to system gettimeofday at the C level while time module grows time.gettimeofday() Python method.
I am not sure I made the best choice defining struct timeval on systems without gettimeofday. Maybe it is better to do
typedef struct timeval _PyTime_timeval;
on systems with gettimeofday and equivalent explicit definition on systems without.
The cast selection logic for time_t is a bit of a hack as well, but this is the best I could come up with without resorting to configure tests. Unless this breaks on a known platform, I would rather commit this as is and have a separate project to clean up cross-platform time handling later.
This patch builds on refactoring started in .
Author: Raymond Hettinger (rhettinger) *
Date: 2010-06-25 17:56
It would be great if Tim Peters could be consulted about some of these ideas. He put a great deal of thought into the API, what should be included and what should be excluded.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-06-25 18:13
I would very much appreciate Tim's input on datetime issues. This particular issue is fairly minor, but Tim's expertise will be invaluable for anything timezone related. I do appreciate the incredible amount of brainpower that went into the design of datetime module and I think the result is very good. There is only one thing that I would like to improve - eliminate the need to reach out to the time module when working with the datetime instances. This patch is a small step to removing the dependency at the implementation level.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-06-29 19:58
To keep things clean, would it be possible to separate this into two patches, one containing the refactoring and one for the newly exposed gettimeofday?
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-06-29 20:27
The original patch, gettimeofday.diff was just refactoring. I unlinked it to keep the file list clean, but it is still available:
http://bugs.python.org/file17766/gettimeofday.diff
I decided to expose time.gettimeofday() in the same patch mostly in order to be able to better explain the consequences of the change such as date.today() now being equivalent to date.fromtimestamp(time.gettimeofday()[0]) rather than date.fromtimestamp(time.time()).
Author: Antoine Pitrou (pitrou) *
Date: 2010-07-12 16:07
Instead of being defined in _time.h, perhaps the new API function should be provided by core Python? It would probably help in certain threading primitives.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-07-12 17:01
On Mon, Jul 12, 2010 at 12:07 PM, Antoine Pitrou <report@bugs.python.org> wrote: ..
Instead of being defined in _time.h, perhaps the new API function should be provided by core Python?
This is an interesting idea, but proposed Py_gettimeofday inherits float_time logic of falling back on ftime and then plain time in case gettimeofday is not available. I am not sure this behavior is always beneficial. I notice that Python/ceval_gil.h has a comment /* We assume all modern POSIX systems have gettimeofday() */, which means it handles windows completely differently and using Py_gettimeofday instead of GETTIMEOFDAY in gil code may not be appropriate.
Author: Antoine Pitrou (pitrou) *
Date: 2010-07-12 17:06
Instead of being defined in _time.h, perhaps the new API function should be provided by core Python?
This is an interesting idea, but proposed Py_gettimeofday inherits float_time logic of falling back on ftime and then plain time in case gettimeofday is not available. I am not sure this behavior is always beneficial.
I would say this is fine as long as it's "documented". The whole point of this API is precisely to avoid having to code the various fallbacks and conversions by yourself. People wanting full control over the code path can just write their own routine.
I notice that Python/ceval_gil.h has a comment /* We assume all modern POSIX systems have gettimeofday() */, which means it handles windows completely differently and using Py_gettimeofday instead of GETTIMEOFDAY in gil code may not be appropriate.
Indeed, the GIL code would probably still use its own code paths. However, other less sensitive code could rely on the new API. For example, it is not critical for lock timeouts to benefit from the full gettimeofday() precision.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-07-12 17:16
On Mon, Jul 12, 2010 at 1:06 PM, Antoine Pitrou <report@bugs.python.org> wrote: ..
Indeed, the GIL code would probably still use its own code paths. However, other less sensitive code could rely on the new API. For example, it is not critical for lock timeouts to benefit from the full gettimeofday() precision.
That may be true, but I would rather proceed in small steps. First expose it in _time.h so that it is shared by time and datetime modulesand then if specific uses are found inside core python, move it to a more appropriate header. Lock timeouts are not a good use case for gettimeofday because POSIX APIs use nonoseconds instead of microseconds.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-07-12 19:26
Assigning back to Alexander; sorry I haven't had time to look at this.
Author: Reid Kleckner (rnk)
Date: 2010-07-13 05:12
The patch looks good to me FWIW.
I would be interested in using this perhaps in , which involves lock timeouts. It may be true that the POSIX API uses nanoseconds, but pythreads only exposes microsecond precision.
In order to use it from the thread module, it need to get moved into Python/. My best guess at the best place to put it would be sysmodule.c, since that already wraps other system calls. Or, you know, we could live a little and make a new file for it. :)
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-07-16 19:33
OK, can someone show me an example of how functions defined in core Python can be made available to extension modules? I thought I could model pytime.h/.c after pymath.h/.c, but the later is not used in extension modules. I must be missing something trivial here.
Author: Antoine Pitrou (pitrou) *
Date: 2010-07-16 19:36
OK, can someone show me an example of how functions defined in core Python can be made available to extension modules? I thought I could model pytime.h/.c after pymath.h/.c, but the later is not used in extension modules. I must be missing something trivial here.
You should just need to #include "pytime.h" from Include/Python.h. Actually, pymath.h is already included from there.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-07-16 19:51
No it must be something else. Attached -fail.diff fails with
Symbol not found: __PyTime_gettimeofday Referenced from: .../build/lib.macosx-10.4-x86_64-3.2-pydebug/datetime.so
even though it is in pytime.o
$ nm ./Python/pytime.o 00000000000009f0 s EH_frame1 0000000000000000 T __PyTime_gettimeofday 0000000000000a08 S __PyTime_gettimeofday.eh U _gettimeofday U _time
Author: Antoine Pitrou (pitrou) *
Date: 2010-07-16 19:57
Le vendredi 16 juillet 2010 à 19:51 +0000, Alexander Belopolsky a écrit :
Alexander Belopolsky <belopolsky@users.sourceforge.net> added the comment:
No it must be something else. Attached -fail.diff fails with
Here it fails earlier:
gcc -pthread -c -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -I. -IInclude -I./Include -DPy_BUILD_CORE -o Python/structmember.o Python/structmember.c Python/pytime.c: In function ‘_PyTime_gettimeofday’: Python/pytime.c:37: erreur: storage size of ‘t’ isn’t known Python/pytime.c:38: attention : implicit declaration of function ‘ftime’ Python/pytime.c:37: attention : unused variable ‘t’ make: *** [Python/pytime.o] Erreur 1 make: *** Attente des tâches non terminées....
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-07-16 19:59
Antoine,
you must be building on Windows. I'll try to guess where ftime is defined and repost the patch.
Author: Georg Brandl (georg.brandl) *
Date: 2010-07-16 20:07
I'd like to help diagnose but I don't know what "Erreur 1" means.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-07-16 20:10
I fixed the ftime issue (I hope), but the build still fails. I did not test on Linux, but I tested on OSX with HAVE_FTIME. Replacing the failing patch.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-07-16 20:24
Removing spurious configure change from the "fail" patch.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-07-16 20:42
It looks like my reluctance to add gettimeofday to core without using it there was well founded. Simply adding a dummy call to _PyTime_gettimeofday in main() fixed the build problem. This is a hack, of course, so I am still looking for suggestions on how to make this work.
=================================================================== --- Modules/python.c (revision 82921) +++ Modules/python.c (working copy) @@ -34,6 +34,8 @@ m = fpgetmask(); fpsetmask(m & ~FP_X_OFL); #endif
- struct timeval tv;
- _PyTime_gettimeofday(&tv); if (!argv_copy || !argv_copy2) { fprintf(stderr, "out of memory\n"); return 1;
Author: Antoine Pitrou (pitrou) *
Date: 2010-07-16 20:56
It looks like my reluctance to add gettimeofday to core without using it there was well founded. Simply adding a dummy call to _PyTime_gettimeofday in main() fixed the build problem.
It's rather strange. I'm sure we have external API functions which never get called in the core. Just a couple of attempts found one candidate:
$ grep -r _PyImport_IsScript Modules/ Objects/ Python/ Include/ Python/import.c:1804:PyAPI_FUNC(int) _PyImport_IsScript(struct filedescr * fd) Include/import.h:43:PyAPI_FUNC(int) _PyImport_IsScript(struct filedescr *);
Author: Antoine Pitrou (pitrou) *
Date: 2010-07-16 21:00
It looks like my reluctance to add gettimeofday to core without using it there was well founded. Simply adding a dummy call to _PyTime_gettimeofday in main() fixed the build problem.
It's rather strange. I'm sure we have external API functions which never get called in the core. Just a couple of attempts found one candidate:
Hmm, after thinking about it, what happens is that the C object file is not used at all, so it's probably optimized away by the linker. In any case, you could use the new API in Python/thread_pthread.h.
Author: Mark Dickinson (mark.dickinson) *
Date: 2010-07-16 21:11
Hmm, after thinking about it, what happens is that the C object file is not used at all, so it's probably optimized away by the linker.
That sounds right. I recall similar problems with pymath.c at one stage: none of the functions it defined was being used in the core, so it didn't get compiled into the python executable and the build failed similarly.
Author: Reid Kleckner (rnk)
Date: 2010-07-16 21:52
Right, it's one of the peculiarities of archive files (I think). When none of an object file's symbols are used from the main program, the object file is dropped on the floor, ie not included. This has bizarre consequences in C++ with static initializers, which get dropped.
On Windows, the PyAPI_FUNC macros should prevent the linker from stripping the datetime stuff.
Jeff Yasskin says you should create a noop function in your object file and call it from PyMain for force linkage of the object file you're building.
Author: Antoine Pitrou (pitrou) *
Date: 2010-07-16 21:57
Jeff Yasskin says you should create a noop function in your object file and call it from PyMain for force linkage of the object file you're building.
Indeed, something like _PyTime_Init(). I don't think Modules/python.c is a good place though, since it means that applications embedding Python won't get the symbol. Perhaps Python/pythonrun.c instead?
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-07-16 21:57
Reid,
I am leaning towards reverting to Plan A (.diff). Would your use case be served well by a _time module exposing C API via a capsule? This what datetime does currently.
Author: Reid Kleckner (rnk)
Date: 2010-07-16 22:20
I'd really rather not try to rely module loading from a threading primitive. :)
I think if you follow Antoine's suggestion of adding _PyTime_Init (which does nothing in the body other than a comment) it should work fine.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-07-17 01:49
It turns out I misunderstood how date.today() worked [1] and .diff introduced significant change in behavior. Bringing timeofday into core rather than _time.c also introduced several complications. As a result it makes sense to start with just internal restructuring: bring gettimeofday into core and reuse it in time.time and datetime.datetime.now. The new patch, issue9079a.diff contains no user visible changes.
[1] "Curious datetime method" http://mail.python.org/pipermail/python-dev/2010-July/102003.html
Author: Reid Kleckner (rnk)
Date: 2010-07-17 16:52
I think you forgot to svn add pytime.c before making the diff.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-07-17 21:22
Indeed. Replacing issue9079a.diff now.
Author: Reid Kleckner (rnk)
Date: 2010-07-21 15:34
pytime.h looks like it got pasted into the file twice. Other than that, it looks good to me and the tests pass on OS X here.
Author: Brian Curtin (brian.curtin) *
Date: 2010-07-21 16:33
issue9079a.diff doesn't compile on Windows - timeval isn't defined. You'd have to include Winsock2.h [0]. Adding something like the following within the HAVE_FTIME block would work... #ifdef MS_WINDOWS #include <Winsock2.h> #endif
I don't currently have time to dig deeper into this stuff, but test_queue hangs. Running the suite without test_queue leaves 31 other failures and something hangs in the background so the test never completes. That was all done with issue9079a.diff applied, plus my winsock include.
[0] http://msdn.microsoft.com/en-us/library/ms740560(VS.85).aspx
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-07-21 18:21
Interesting. As far as I can tell, struct timeval should not be used unless HAVE_GETTIMEOFDAY is defined:
+#ifdef HAVE_GETTIMEOFDAY +typedef struct timeval _PyTime_timeval; +#else +typedef struct {
- time_t tv_sec; /* seconds since Jan. 1, 1970 */
- long tv_usec; /* and microseconds */
+} _PyTime_timeval; +#endif
Brian, where do you get undefined symbol error?
Author: Reid Kleckner (rnk)
Date: 2010-07-21 18:25
I think you used 'struct timeval *' in the function definition instead of '_PyTimeVal *'.
Author: Brian Curtin (brian.curtin) *
Date: 2010-07-21 18:28
Here are the errors I get:
Error 104 error C2037: left of 'tv_sec' specifies undefined struct/union 'timeval' c:\python-dev\py3k\Python\pytime.c 46 pythoncore Error 105 error C2037: left of 'tv_usec' specifies undefined struct/union 'timeval' c:\python-dev\py3k\Python\pytime.c 47 pythoncore
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-07-21 18:34
I think Reid nailed the issue. Fix is coming.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-07-21 18:38
issue9079b.diff should fix the Windows issue.
Author: Brian Curtin (brian.curtin) *
Date: 2010-07-21 18:51
I won't have time to review this, but I can say issue9079b.diff works fine on Windows.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-08-05 16:53
Updated the patch to reflect recent datetimemodule.c renaming to _datetimemodule.c. No other changes between issue9079b.diff and issue9079c.diff. I am going to commit issue9079c.diff soon unless someone wants more time to review.
Author: Alexander Belopolsky (belopolsky) *
Date: 2010-08-05 17:37
Committed issue9079c.diff as r83744. This commit does not provide time.gettimeofday() and therefore does not close the issue.
Author: STINNER Victor (vstinner) *
Date: 2012-03-14 00:49
The new patch, .diff exposes gettimeofday as time.gettimeofday() returning (sec, usec) pair.
A tuple is not the preferred type for a timestamp: Python uses float and is not going to use something different (the PEP 410 was just rejected).
I don't see what we need a new function: there is always time.time().
I'm closing this issue because I consider it as done.
History
Date
User
Action
Args
2022-04-11 14:57:02
admin
set
github: 53325
2012-03-14 00:49:07
vstinner
set
status: open -> closed
resolution: fixed
messages: +
2010-12-15 20:38:39
pitrou
unlink
2010-10-05 17:32:20
belopolsky
set
nosy: + vstinner
2010-08-05 17:37:07
belopolsky
set
keywords: - patch, needs review
messages: +
stage: commit review -> needs patch
2010-08-05 16:53:40
belopolsky
set
files: + issue9079c.diff
messages: +
2010-07-21 18:51:18
brian.curtin
set
messages: +
2010-07-21 18:38:31
belopolsky
set
files: + issue9079b.diff
messages: +
2010-07-21 18:34:32
belopolsky
set
messages: +
2010-07-21 18:28:55
brian.curtin
set
messages: +
2010-07-21 18:25:31
rnk
set
messages: +
2010-07-21 18:21:35
belopolsky
set
messages: +
2010-07-21 16:33:52
brian.curtin
set
nosy: + brian.curtin
messages: +
2010-07-21 15:34:36
rnk
set
messages: +
2010-07-18 15:04:11
rnk
link
2010-07-17 21:22:51
belopolsky
set
files: - issue9079a.diff
2010-07-17 21:22:42
belopolsky
set
files: + issue9079a.diff
messages: +
2010-07-17 16:52:41
rnk
set
messages: +
2010-07-17 01:49:31
belopolsky
set
files: + issue9079a.diff
messages: +
2010-07-16 22:20:40
rnk
set
messages: +
2010-07-16 21:57:57
belopolsky
set
messages: +
2010-07-16 21:57:18
pitrou
set
messages: +
2010-07-16 21:52:21
rnk
set
messages: +
2010-07-16 21:11:28
mark.dickinson
set
messages: +
2010-07-16 21:00:43
pitrou
set
messages: +
2010-07-16 20:56:06
pitrou
set
messages: +
2010-07-16 20:42:52
belopolsky
set
files: + issue9079-builds.diff
messages: +
2010-07-16 20:24:14
belopolsky
set
files: - issue9079-fail.diff
2010-07-16 20:24:06
belopolsky
set
files: + issue9079-fail.diff
messages: +
2010-07-16 20:10:18
belopolsky
set
files: - issue9079-fail.diff
2010-07-16 20:10:09
belopolsky
set
files: + issue9079-fail.diff
messages: +
2010-07-16 20:07:32
georg.brandl
set
nosy: + georg.brandl
messages: +
2010-07-16 19:59:41
belopolsky
set
messages: +
2010-07-16 19:57:09
pitrou
set
messages: +
2010-07-16 19:52:23
belopolsky
set
files: + issue9079-fail.diff
2010-07-16 19:51:38
belopolsky
set
messages: +
2010-07-16 19:36:43
pitrou
set
messages: +
2010-07-16 19:33:14
belopolsky
set
messages: +
2010-07-14 14:56:58
belopolsky
set
nosy: + tim.golden
2010-07-13 05:12:11
rnk
set
nosy: + rnk
messages: +
2010-07-12 20:35:20
mark.dickinson
set
assignee: mark.dickinson -> belopolsky
2010-07-12 19:26:56
mark.dickinson
set
messages: +
2010-07-12 17:16:43
belopolsky
set
messages: +
2010-07-12 17:06:12
pitrou
set
messages: +
2010-07-12 17:01:14
belopolsky
set
messages: +
2010-07-12 16:07:10
pitrou
set
nosy: + pitrou
messages: +
2010-06-29 20:27:35
belopolsky
set
messages: +
2010-06-29 19:58:58
mark.dickinson
set
messages: +
2010-06-25 18:13:24
belopolsky
set
nosy: + tim.peters
messages: +
2010-06-25 17:56:48
rhettinger
set
nosy: + rhettinger
messages: +
2010-06-25 15:12:40
belopolsky
link
2010-06-25 14:09:38
belopolsky
set
title: Make gettimeofday available in datetime module -> Make gettimeofday available in time module
messages: +
assignee: belopolsky -> mark.dickinson
type: enhancement
stage: patch review -> commit review
2010-06-25 02:36:11
belopolsky
set
files: - gettimeofday.diff
2010-06-25 02:35:06
belopolsky
set
files: + issue9079.diff
messages: +
2010-06-25 00:52:32
belopolsky
create