homepage

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.

The new patch, issue9079.diff exposes gettimeofday as time.gettimeofday() returning (sec, usec) pair.

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.

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.

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.

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?

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

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.

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.

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

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.

Assigning back to Alexander; sorry I haven't had time to look at this.

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

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.

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

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

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

Antoine,
you must be building on Windows. I'll try to guess where ftime is defined and repost the patch.

I'd like to help diagnose but I don't know what "Erreur 1" means.

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.

Removing spurious configure change from the "fail" patch.

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;

> 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 *);

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

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

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.

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

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.

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.

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 

I think you forgot to svn add pytime.c before making the diff.

Indeed. Replacing issue9079a.diff now.

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.

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 

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?

I think you used 'struct timeval *' in the function definition instead of '_PyTimeVal *'.

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

I think Reid nailed the issue. Fix is coming.

issue9079b.diff should fix the Windows issue.

I won't have time to review this, but I can say issue9079b.diff works fine on Windows.

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.

Committed issue9079c.diff as r83744. This commit does not provide time.gettimeofday() and therefore does not close the issue.

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