classification
Title: Make gettimeofday available in time module
Type: enhancement Stage: needs patch
Components: Extension Modules Versions: Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: belopolsky Nosy List: belopolsky, brian.curtin, georg.brandl, mark.dickinson, pitrou, rhettinger, rnk, tim.golden, tim.peters, vstinner
Priority: normal Keywords:

Created on 2010-06-25 00:52 by belopolsky, last changed 2012-03-14 00:49 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
issue9079.diff belopolsky, 2010-06-25 02:35
issue9079-fail.diff belopolsky, 2010-07-16 20:24 this fails to build
issue9079-builds.diff belopolsky, 2010-07-16 20:42 this builds
issue9079a.diff belopolsky, 2010-07-17 21:22 Patch for py3k branch, revision 82936.
issue9079b.diff belopolsky, 2010-07-21 18:38
issue9079c.diff belopolsky, 2010-08-05 16:53
Messages (43)
msg108575 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) 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.
msg108577 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-06-25 02:35
The new patch, issue9079.diff exposes gettimeofday as time.gettimeofday() returning (sec, usec) pair.
msg108599 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) 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 issue9012.
msg108612 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) 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.
msg108614 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) 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.
msg108942 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) 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?
msg108947 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) 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()).
msg110104 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) 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.
msg110112 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) 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.
msg110113 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) 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.
msg110114 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) 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.
msg110125 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-07-12 19:26
Assigning back to Alexander;  sorry I haven't had time to look at this.
msg110158 - (view) Author: Reid Kleckner (rnk) (Python committer) Date: 2010-07-13 05:12
The patch looks good to me FWIW.

I would be interested in using this perhaps in issue8844, 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.  :)
msg110475 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) 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.
msg110476 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) 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.
msg110478 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-07-16 19:51
No it must be something else.  Attached issue9079-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
msg110479 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) 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 issue9079-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....
msg110480 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) 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.
msg110482 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2010-07-16 20:07
I'd like to help diagnose but I don't know what "Erreur 1" means.
msg110483 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) 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.
msg110488 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-07-16 20:24
Removing spurious configure change from the "fail" patch.
msg110491 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) 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;
msg110493 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) 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 *);
msg110496 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) 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.
msg110498 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) 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.
msg110508 - (view) Author: Reid Kleckner (rnk) (Python committer) 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.
msg110509 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) 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?
msg110510 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-07-16 21:57
Reid,

I am leaning towards reverting to Plan A (issue9079.diff).  Would your use case be served well by a _time module exposing C API via a capsule?  This what datetime does currently.
msg110516 - (view) Author: Reid Kleckner (rnk) (Python committer) 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.
msg110535 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-07-17 01:49
It turns out I misunderstood how date.today() worked [1] and issue9079.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
msg110583 - (view) Author: Reid Kleckner (rnk) (Python committer) Date: 2010-07-17 16:52
I think you forgot to svn add pytime.c before making the diff.
msg110603 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-07-17 21:22
Indeed.  Replacing issue9079a.diff now.
msg111077 - (view) Author: Reid Kleckner (rnk) (Python committer) 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.
msg111085 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) 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
msg111095 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) 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?
msg111096 - (view) Author: Reid Kleckner (rnk) (Python committer) Date: 2010-07-21 18:25
I think you used 'struct timeval *' in the function definition instead of '_PyTimeVal *'.
msg111098 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) 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
msg111099 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-07-21 18:34
I think Reid nailed the issue.  Fix is coming.
msg111102 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-07-21 18:38
issue9079b.diff should fix the Windows issue.
msg111105 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-07-21 18:51
I won't have time to review this, but I can say issue9079b.diff works fine on Windows.
msg113000 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) 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.
msg113001 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) 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.
msg155698 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2012-03-14 00:49
> The new patch, issue9079.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
2012-03-14 00:49:07vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg155698
2010-12-15 20:38:39pitrouunlinkissue8844 dependencies
2010-10-05 17:32:20belopolskysetnosy: + vstinner
2010-08-05 17:37:07belopolskysetkeywords: - patch, needs review

messages: + msg113001
stage: commit review -> needs patch
2010-08-05 16:53:40belopolskysetfiles: + issue9079c.diff

messages: + msg113000
2010-07-21 18:51:18brian.curtinsetmessages: + msg111105
2010-07-21 18:38:31belopolskysetfiles: + issue9079b.diff

messages: + msg111102
2010-07-21 18:34:32belopolskysetmessages: + msg111099
2010-07-21 18:28:55brian.curtinsetmessages: + msg111098
2010-07-21 18:25:31rnksetmessages: + msg111096
2010-07-21 18:21:35belopolskysetmessages: + msg111095
2010-07-21 16:33:52brian.curtinsetnosy: + brian.curtin
messages: + msg111085
2010-07-21 15:34:36rnksetmessages: + msg111077
2010-07-18 15:04:11rnklinkissue8844 dependencies
2010-07-17 21:22:51belopolskysetfiles: - issue9079a.diff
2010-07-17 21:22:42belopolskysetfiles: + issue9079a.diff

messages: + msg110603
2010-07-17 16:52:41rnksetmessages: + msg110583
2010-07-17 01:49:31belopolskysetfiles: + issue9079a.diff

messages: + msg110535
2010-07-16 22:20:40rnksetmessages: + msg110516
2010-07-16 21:57:57belopolskysetmessages: + msg110510
2010-07-16 21:57:18pitrousetmessages: + msg110509
2010-07-16 21:52:21rnksetmessages: + msg110508
2010-07-16 21:11:28mark.dickinsonsetmessages: + msg110498
2010-07-16 21:00:43pitrousetmessages: + msg110496
2010-07-16 20:56:06pitrousetmessages: + msg110493
2010-07-16 20:42:52belopolskysetfiles: + issue9079-builds.diff

messages: + msg110491
2010-07-16 20:24:14belopolskysetfiles: - issue9079-fail.diff
2010-07-16 20:24:06belopolskysetfiles: + issue9079-fail.diff

messages: + msg110488
2010-07-16 20:10:18belopolskysetfiles: - issue9079-fail.diff
2010-07-16 20:10:09belopolskysetfiles: + issue9079-fail.diff

messages: + msg110483
2010-07-16 20:07:32georg.brandlsetnosy: + georg.brandl
messages: + msg110482
2010-07-16 19:59:41belopolskysetmessages: + msg110480
2010-07-16 19:57:09pitrousetmessages: + msg110479
2010-07-16 19:52:23belopolskysetfiles: + issue9079-fail.diff
2010-07-16 19:51:38belopolskysetmessages: + msg110478
2010-07-16 19:36:43pitrousetmessages: + msg110476
2010-07-16 19:33:14belopolskysetmessages: + msg110475
2010-07-14 14:56:58belopolskysetnosy: + tim.golden
2010-07-13 05:12:11rnksetnosy: + rnk
messages: + msg110158
2010-07-12 20:35:20mark.dickinsonsetassignee: mark.dickinson -> belopolsky
2010-07-12 19:26:56mark.dickinsonsetmessages: + msg110125
2010-07-12 17:16:43belopolskysetmessages: + msg110114
2010-07-12 17:06:12pitrousetmessages: + msg110113
2010-07-12 17:01:14belopolskysetmessages: + msg110112
2010-07-12 16:07:10pitrousetnosy: + pitrou
messages: + msg110104
2010-06-29 20:27:35belopolskysetmessages: + msg108947
2010-06-29 19:58:58mark.dickinsonsetmessages: + msg108942
2010-06-25 18:13:24belopolskysetnosy: + tim.peters
messages: + msg108614
2010-06-25 17:56:48rhettingersetnosy: + rhettinger
messages: + msg108612
2010-06-25 15:12:40belopolskylinkissue1578643 dependencies
2010-06-25 14:09:38belopolskysettitle: Make gettimeofday available in datetime module -> Make gettimeofday available in time module
messages: + msg108599

assignee: belopolsky -> mark.dickinson
type: enhancement
stage: patch review -> commit review
2010-06-25 02:36:11belopolskysetfiles: - gettimeofday.diff
2010-06-25 02:35:06belopolskysetfiles: + issue9079.diff

messages: + msg108577
2010-06-25 00:52:32belopolskycreate