classification
Title: Add pure Python implementation of time module to CPython
Type: enhancement Stage: resolved
Components: Extension Modules, Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: belopolsky Nosy List: alanjds, amaury.forgeotdarc, anacrolix, belopolsky, brett.cannon, brian.curtin, daniel.urban, davidfraser, eric.araujo, giampaolo.rodola, haypo, lemburg, mark.dickinson, pitrou, r.david.murray, rhettinger, techtonik, tim.peters
Priority: normal Keywords:

Created on 2010-08-06 04:33 by belopolsky, last changed 2012-03-22 10:13 by anacrolix. This issue is now closed.

Messages (13)
msg113073 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-08-06 04:33
The original RFE at issue 7989 was:

"""
After discussion on numerous issues, python-dev, and here at the PyCon sprints, it seems to be a good idea to move timemodule.c to _timemodule.c and convert as much as possible into pure Python. The same change seems good for datetime.c as well.
"""

See msg99774.  I have changed issue 7989 to cover datetime only because I argued that as a thin wrapper around C library calls, this module is an exception to the general rule that pure python implementations are a good idea.  See msg107303.

No I realize that in order to break circular dependency between time and datetime modules, it will be helpful to create an _time module that would provide lower than time module access to system facilities and datetime and time modules would be use _time module to implement higher level interfaces either in C or in Python.

I believe _time module should become the home of the gettimeofday() method and pure python implementation of time.time() will be

def time()
   s, us = _time.gettimeofday()
   return s + 1e-6 * us


Similarly time.sleep() can be implemented in terms of lower level POSIX nanosleep() method.

Lower level localtime() function can provide access to tm_zone and tm_gmtoff members of struct tm (where available) without concerns about backward compatibility.
msg113084 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2010-08-06 07:44
> I believe _time module should become the home of the gettimeofday() method and pure python implementation of time.time() will be
> 
> def time()
>    s, us = _time.gettimeofday()
>    return s + 1e-6 * us
> 
> 
> Similarly time.sleep() can be implemented in terms of lower level POSIX nanosleep() method.
> 
> Lower level localtime() function can provide access to tm_zone and tm_gmtoff members of struct tm (where available) without concerns about backward compatibility.

Just for understanding:

Why are you calling the ticket "*Add* pure Python implementation of time
module to CPython" when you appear to be after *replacing* the C
implementation of the time module with a Python version ?

The same argument as for the datetime module applies: you can *add*
a compatible Python version of the same module for other Python
implementations to use, but undoing the work that has been done
in order to provide a faster implementation of the Python version
is a no-go.

Both datetime and time module functionalities need to be as fast as
possible, since they are used a lot in Python code. That was the
main reason for having a C implementation of the datetime and time
modules.

Python C function calls are still a lot faster than Python function
calls. You can't just replace a C function call with a Python one
without taking this into account. For these modules, it's not just
the API compatibility that matters, performance is just as
relevant and I don't really see a point in making CPython slower
just to make maintenance of stdlib modules that are not needed by
CPython easier.
msg113085 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2010-08-06 09:24
These changes are very aggressive and may be doing more harm than good.
msg113103 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-08-06 14:26
On Fri, Aug 6, 2010 at 3:44 AM, Marc-Andre Lemburg
<report@bugs.python.org> wrote:
..
> Why are you calling the ticket "*Add* pure Python implementation of time
> module to CPython" when you appear to be after *replacing* the C
> implementation of the time module with a Python version ?
>

I have deliberately made the title similar to issue 7989 so that it is
clear that the idea is the same: add python implementations of time
module functions which are overridden by existing C implementations in
CPython.  The only difference with datetime is that for Python
implementation to work, it needs access to some system facilities. The
datetime.py module needs access to some of these facilities as well,
but currently it works around this problem by importing them from time
module.

The current situation has several problems:

1. Datetime.py time source (time.time()) represents time as a floating
point number which leads to system dependent behavior and introduces
floating point operations where they are not needed.

2. Datetime.strftime function is restricted to years >= 1900 even on
platforms where system strftime is perfectly good over full range of
datetime. This is done expressly because semantics of time.strftime
dictate such behavior.  See issue 1777412.

3. The datetime module could benefit from access to tm_zone and
tm_gmtoff components of the tm structure, but introducing those in the
output of time.localtime() would require either hidden member hackery
or loss of backwards compatibility.  See issues #1647654, #7662, and
#9527.

> The same argument as for the datetime module applies: you can *add*
> a compatible Python version of the same module for other Python
> implementations to use, but undoing the work that has been done
> in order to provide a faster implementation of the Python version
> is a no-go.
>

I completely agree.  Actually my outline in the first post is
incomplete.  What I would like to do is:

1. Rename timemodule.c to _timemodule.c
2. Convert non-module _time.c (home of code shared between time and
datetime C implementations) to a proper C extension.  See issue 9012,
msg109221 for description of the problem with the current strategy.
For luck of better name, I'll call the resulting module _basictime, so
 _time.c will get renamed to _basictimemodule.c.
3. Make _basictime module expose C and Python API to basic time
facilities: integer-valued time sources (gettimeofday, clock, etc.),
integer-based sleep method (say nanosleep), thin wrappers around
system strftime and strptime functions and tzset method.
4. No changes will be done to timemodule.c other than renaming. The
new time.py will import _basictime to implement it's methods in
python, but will end with from _time import *.  The _time module will
not depend on _basictime.
5. In the future, but not as a part of this proposal, datetime C and
python implementations can start using _basictime for low level access
rather than time.

> Both datetime and time module functionalities need to be as fast as
> possible, since they are used a lot in Python code. That was the
> main reason for having a C implementation of the datetime and time
> modules.
>

Absolutely.  In fact this proposal will open the door to implementing
*faster* C API to basic time facilities.  I do want _basictime to
properly expose its C API similarly to the way datetime module already
does.   The current situation is somewhat cheating: datetime module
advertises fast C API, but under the hood imports time module and
makes python calls to its methods.

BTW, this brings a point that I think I missed when I introduced
datetime.py.  Should PyDateTime_CAPSULE_NAME be changed from
"datetime.datetime_CAPI" to  "_datetime.datetime_CAPI" in order to
eliminate even small overhead that loading datetime rather than
_datetime carries?

> Python C function calls are still a lot faster than Python function
> calls. You can't just replace a C function call with a Python one
> without taking this into account. For these modules, it's not just
> the API compatibility that matters, performance is just as
> relevant and I don't really see a point in making CPython slower
> just to make maintenance of stdlib modules that are not needed by
> CPython easier.

Brett addressed this point responding to my own concerns in msg107295
and msg108047.  I will just repeat: this proposal will make no changes
to the functions that CPython users will import from time module.
msg113742 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-08-13 02:26
> 1. Datetime.py time source (time.time()) represents time as 
> a floating point number which leads to system dependent behavior
> and introduces floating point operations where they are not needed.

Why not introducing a new function in time module? Other people may benefit from this.

> 3. The datetime module could benefit from access to tm_zone 
> and tm_gmtoff components of the tm structure, but introducing 
> those in the output of time.localtime() would require either 
> hidden member hackery or loss of backwards compatibility.
> See issues #1647654, #7662, and #9527.

Why not creating a new function giving access to all tm attributes? Eg. time.localtime_gmaware()?

> 4. No changes will be done to timemodule.c other than renaming

What about time_strftime()? It is 170 lines long: will it be moved to _basictime.c? You have to keep the code filling the "struct tm" structure in (_)timemodule.c.
msg118023 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-10-05 17:52
On Thu, Aug 12, 2010 at 10:26 PM, STINNER Victor <report@bugs.python.org> wrote:
..
>> 1. Datetime.py time source (time.time()) represents time as
>> a floating point number which leads to system dependent behavior
>> and introduces floating point operations where they are not needed.
>
> Why not introducing a new function in time module? Other people may benefit from this.
>
I agree.  See issue 9079.

We can do that.  I'll experiment with this approach within issue 9527.

>> 4. No changes will be done to timemodule.c other than renaming
>
> What about time_strftime()? It is 170 lines long: will it be moved to _basictime.c? You have to keep
> the code filling the "struct tm" structure in (_)timemodule.c.

No, I don't want  time_strftime in _basictime.  I want
datetime_strftime to be independently implemented and freed of legacy
restrictions on the year range.   The _basictime module should include
a very simple wrapper around system strftime if we want to keep using
it in datetime.py, but it would be best to have complete pure python
implementation of both strftime and strptime.
msg123003 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-12-01 18:43
BDFL and python-dev were opposed to this idea.
msg141874 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-08-10 16:38
@alanjds: Why do you add Python 2.7 version to this issue?
msg141875 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011-08-10 17:18
@Victor it doesn't really matter why since it is an incorrect classification. This issue in no way involves Python 2.7 since we will not backport any modules to Python 2.7.
msg141877 - (view) Author: Alan Justino (alanjds) Date: 2011-08-10 17:29
@haypo: Because it affects version 2.7 too.

@Victor: Even since we will not backport any modules to Python 2.7, is not worth to sign that this affects it too? Even wontfix or rejected, it affects, does not?

Have I made something wrong? Sorry to be so newbie about this.

Long explanation:

Is not easy to change/extend datetime, as stated by several other before. Searching the bugs for 2.x only does not shows that someone else already care about this issue AND that it exists at 2.x too.

I am getting a hard time trying to do some BDD with c-based datetime because I cannot mock it easily to force datetime.datetime.now() to return a desired value, making almost impossible to test time-based code, like the accounting system that I am refactoring right now.

Another solution would be to "open" C-based classes to allow modification, that is not planned to be into near future Python versions AFAIK.

I came to fill a new bug with all this, but then found this one that is already mature enough, with even BDFL and python-dev opinion.

Then my (maybe pointless) plan was to gather more arguments favor to change into python-based datetime, but you replied first...
msg141878 - (view) Author: Alan Justino (alanjds) Date: 2011-08-10 17:31
(noticed a typo at the start of my last msg: @Victor should be @brett.cannon)
msg141880 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2011-08-10 17:39
Alan Justino wrote:
> 
> I am getting a hard time trying to do some BDD with c-based datetime because I cannot mock it easily to force datetime.datetime.now() to return a desired value, making almost impossible to test time-based code, like the accounting system that I am refactoring right now.

It's usually better to use a central helper get_current_time() in
the application, than to use datetime.now() and others
directly.
msg141911 - (view) Author: √Čric Araujo (eric.araujo) * (Python committer) Date: 2011-08-11 16:08
Alan: the Versions field is used to mark versions that will get a patch, not all versions affected.
History
Date User Action Args
2012-03-22 10:13:40anacrolixsetnosy: + anacrolix
2011-08-11 16:08:09eric.araujosetmessages: + msg141911
2011-08-10 17:39:52lemburgsetmessages: + msg141880
2011-08-10 17:31:44alanjdssetmessages: + msg141878
2011-08-10 17:29:50alanjdssetmessages: + msg141877
2011-08-10 17:18:35brett.cannonsetmessages: + msg141875
versions: + Python 3.3, - Python 2.7, Python 3.2
2011-08-10 16:38:23hayposetmessages: + msg141874
2011-08-09 23:52:34alanjdssetnosy: + alanjds

versions: + Python 2.7
2010-12-01 18:43:25belopolskysetstatus: open -> closed
resolution: rejected
messages: + msg123003

stage: resolved
2010-10-05 17:52:23belopolskysetmessages: + msg118023
2010-08-13 02:26:02hayposetmessages: + msg113742
2010-08-06 14:27:00belopolskysetmessages: + msg113103
2010-08-06 09:24:05rhettingersetmessages: + msg113085
2010-08-06 07:44:17lemburgsetmessages: + msg113084
2010-08-06 04:33:21belopolskycreate