Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pure Python implementation of time module to CPython #53737

Closed
abalkin opened this issue Aug 6, 2010 · 13 comments
Closed

Add pure Python implementation of time module to CPython #53737

abalkin opened this issue Aug 6, 2010 · 13 comments
Assignees
Labels
extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@abalkin
Copy link
Member

abalkin commented Aug 6, 2010

BPO 9528
Nosy @malemburg, @tim-one, @brettcannon, @rhettinger, @amauryfa, @mdickinson, @abalkin, @pitrou, @vstinner, @giampaolo, @merwok, @bitdancer, @briancurtin, @durban, @alanjds, @anacrolix

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = 'https://github.com/abalkin'
closed_at = <Date 2010-12-01.18:43:25.796>
created_at = <Date 2010-08-06.04:33:20.317>
labels = ['extension-modules', 'type-feature', 'library']
title = 'Add pure Python implementation of time module to CPython'
updated_at = <Date 2012-03-22.10:13:40.766>
user = 'https://github.com/abalkin'

bugs.python.org fields:

activity = <Date 2012-03-22.10:13:40.766>
actor = 'anacrolix'
assignee = 'belopolsky'
closed = True
closed_date = <Date 2010-12-01.18:43:25.796>
closer = 'belopolsky'
components = ['Extension Modules', 'Library (Lib)']
creation = <Date 2010-08-06.04:33:20.317>
creator = 'belopolsky'
dependencies = []
files = []
hgrepos = []
issue_num = 9528
keywords = []
message_count = 13.0
messages = ['113073', '113084', '113085', '113103', '113742', '118023', '123003', '141874', '141875', '141877', '141878', '141880', '141911']
nosy_count = 18.0
nosy_names = ['lemburg', 'tim.peters', 'brett.cannon', 'rhettinger', 'amaury.forgeotdarc', 'mark.dickinson', 'davidfraser', 'belopolsky', 'pitrou', 'vstinner', 'techtonik', 'giampaolo.rodola', 'eric.araujo', 'r.david.murray', 'brian.curtin', 'daniel.urban', 'alanjds', 'anacrolix']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue9528'
versions = ['Python 3.3']

@abalkin
Copy link
Member Author

abalkin commented Aug 6, 2010

The original RFE at bpo-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 bpo-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.

@abalkin abalkin self-assigned this Aug 6, 2010
@abalkin abalkin added extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Aug 6, 2010
@malemburg
Copy link
Member

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.

@rhettinger
Copy link
Contributor

These changes are very aggressive and may be doing more harm than good.

@abalkin
Copy link
Member Author

abalkin commented Aug 6, 2010

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 bpo-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 bpo-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 bpo-1647654, bpo-7662, and
    bpo-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 bpo-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.

@vstinner
Copy link
Member

  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.

  1. 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 bpo-1647654, bpo-7662, and bpo-9527.

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

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

@abalkin
Copy link
Member Author

abalkin commented Oct 5, 2010

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

We can do that. I'll experiment with this approach within bpo-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.

@abalkin
Copy link
Member Author

abalkin commented Dec 1, 2010

BDFL and python-dev were opposed to this idea.

@abalkin abalkin closed this as completed Dec 1, 2010
@vstinner
Copy link
Member

@alanjds: Why do you add Python 2.7 version to this issue?

@brettcannon
Copy link
Member

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

@alanjds
Copy link
Mannequin

alanjds mannequin commented Aug 10, 2011

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

@alanjds
Copy link
Mannequin

alanjds mannequin commented Aug 10, 2011

(noticed a typo at the start of my last msg: @victor should be @brett.cannon)

@malemburg
Copy link
Member

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.

@merwok
Copy link
Member

merwok commented Aug 11, 2011

Alan: the Versions field is used to mark versions that will get a patch, not all versions affected.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

6 participants