Create Lib/_pydatetime.py file to optimize "import datetime" when _datetime is available

@vstinner

BPO	40799
Nosy	@vstinner, @ezio-melotti, @serhiy-storchaka, @pganssle, @shihai1991
PRs	bpo-40799: Add _pydatetime module (Python datetime impl) #20472

^{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 = None
closed_at = None
created_at = <Date 2020-05-28.00:06:58.019>
labels = ['library', '3.10']
title = 'Create Lib/_pydatetime.py file to optimize "import datetime" when _datetime is available'
updated_at = <Date 2020-06-26.14:58:45.408>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2020-06-26.14:58:45.408>
actor = 'p-ganssle'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2020-05-28.00:06:58.019>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 40799
keywords = ['patch']
message_count = 8.0
messages = ['370153', '370165', '370216', '370222', '370223', '370584', '372425', '372427']
nosy_count = 5.0
nosy_names = ['vstinner', 'ezio.melotti', 'serhiy.storchaka', 'p-ganssle', 'shihai1991']
pr_nums = ['20472']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue40799'
versions = ['Python 3.10']

PR: gh-84976: Add _pydatetime module (Python datetime impl) #99090

PR: GH-84976: Create Lib/_pydatetime to optimize import datetime when C implementation is available #103637

PR: gh-84976: Add back UTC to datetime.__all__ #104920

PR: [3.12] gh-84976: Add back UTC to datetime.__all__ (GH-104920) #106019

I don't have the bandwidth to work on this issue, so I just close it.

I don't think we need to close this just because you won't be fixing it yourself. It is still a valid issue

It is still a valid issue

Well, I made a first attempt in 2020: PR #20472 which it went nowhere.

Let me re-create the change: PR #99090. It's basically the same one, but I made tiny doc changes (ex: use GitHub issue number, not bpo number) and datetimetester.py tolerates that _pydatetime.py doesn't exist (use datetime in this case).

I ran two benchmarks.

Startup time:

env/bin/python -m pyperf command -v -o pydatetime.json -- ./python -IsS -c 'import datetime'

Import time:

env/bin/python -m pyperf timeit -v -o import_change.json -s 'import sys' 'import datetime; del sys.modules["datetime"]; del sys.modules["_datetime"]; del datetime'

Results:

Startup time: Mean +- std dev: [startup_ref] 6.93 ms +- 0.26 ms -> [startup_change] 6.12 ms +- 0.47 ms: 1.13x faster
Import time: Mean +- std dev: [import_ref] 688 us +- 17 us -> [import_change] 133 us +- 12 us: 5.18x faster

IMO the speedup is quite interesting.

You need to add del sys.modules["_pydatetime"] and the same for other indirectly imported modules (math, operator, _operator).

The biggest advantage I see to moving datetime into its own folder is that it gives us a lot more freedom to expand into smaller sub-modules in the future.

I don't think that this constraint (convert datetime into a package) should only this optimization. For me, it can be done later. Honestly, I'm not convinced that converting datetime into a module right now brings any benefits. I prefer a simple .py file (a module, not a package). I don't expect major datetime redesign soon.

If tomorrow, there is strong pressure for large datetime feature which requires creating a package, sure, we can consider that.

Today, Python stdlib is mostly flat and IMO it's a good thing and also a deliberate choice.

Ok, I wrote a more elaborated script to measure import datetime:

import pyperf
import sys


def bench_import_datetime(modules):
    import datetime

    del modules["datetime"]
    del modules["_datetime"]
    modules.pop("time", None)
    modules.pop("math", None)
    modules.pop("operator", None)
    modules.pop("_operator", None)
    modules.pop("_pydatetime", None)

    del datetime

runner = pyperf.Runner()
del pyperf

runner.bench_func('import', bench_import_datetime, sys.modules)

Result (main => PR): Mean +- std dev: [bench_ref] 1.07 ms +- 0.02 ms -> [bench_change] 116 us +- 5 us: 9.20x faster.

So I think on the mailing list we decided that switching to just _pydatetime was fine, but the blocker last time we tried to do this was now making sure that miss-islington would be able to do backports. I can't exactly remember where we left off on that — whether it's the case that the change wouldn't break miss-islington and if it would how big of a problem that is.

I think I vaguely recall it being the case that datetime.py has had very few backports in the last few years — almost all of the code change in datetime is coming from new features that don't get backported rather than bugs in existing features, so if we're looking at 2-3 manual backports per year for the next few years it's probably worth doing this even if it breaks miss-islington, but obviously if there is a way to do it that does not break miss-islington, it's preferable to do it that way.

I'm not sure how to test the miss-islington thing because I'm not sure how easy it is to point miss islington at a different repo and say, "Treat this like it's CPython main, and this other branch like it's 3.11" or whatever. We can try using the same git commands miss-islington does as a first pass check, but before merge I'd be way more comfortable if we could see an actual miss-islington instance creating a backport.

By default, git log Lib/_pydatetime.py only shows my commit as if I created it from scratch. But git log --follow Lib/_pydatetime.py works as expected and shows 81 commits up to:

commit cf86e368ebd17e10f68306ebad314eea31daaa1e
Author: Alexander Belopolsky <alexander.belopolsky@gmail.com>
Date:   Fri Jul 23 19:25:47 2010 +0000

    Issue #7989: Added pure python implementation of the datetime module.

For miss-islington, I did a test locally.

I create a local bugfix from my PR pydatetime branch:

$ git switch -c pydatetime_bugfix
$ vim
$ git diff
diff --git a/Lib/_pydatetime.py b/Lib/_pydatetime.py
index fb1e139379..d1a735e485 100644
--- a/Lib/_pydatetime.py
+++ b/Lib/_pydatetime.py
@@ -2373,7 +2373,7 @@ def tzname(self, dt):
         if isinstance(dt, datetime) or dt is None:
             if self._name is None:
                 return self._name_from_offset(self._offset)
-            return self._name
+            return self._name + "bugfix"
         raise TypeError("tzname() argument must be a datetime instance"
                         " or None")

$ git ci -a -m bugfix
[pydatetime_bugfix 2a89c2221b] bugfix
 1 file changed, 1 insertion(+), 1 deletion(-)

Backport the bugfix from main to the 3.11 branch:

# move the the 3.11 branch
$ cd ../3.11
$ git switch -c pydatetime_bugfix311 
$ git cherry-pick -x pydatetime_bugfix
CONFLICT (modify/delete): Lib/_pydatetime.py deleted in HEAD and modified in 2a89c2221b (bugfix).  Version 2a89c2221b (bugfix) of Lib/_pydatetime.py left in tree.
error: could not apply 2a89c2221b... bugfix
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

$ git status
On branch pydatetime_bugfix311
You are currently cherry-picking commit 2a89c2221b.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	deleted by us:   Lib/_pydatetime.py

no changes added to commit (use "git add" and/or "git commit -a")

Well. It doesn't work as expected. git cherry-pick -x is unable to understand that changes in _pydatetime.py should be backported to datetime.py.

Maybe the problem is that in my PR, datetime.py is renamed to _pydatetime.py... but at the same time, datetime.py is recreated with a new content. Maybe at the end, Git doesn't understand that it's a rename.

Example if I recreated my PR quickly (first steps) from my existing main (reference) and pydatetime (PR) branches:

$ git switch -c clean_pydatetime  main
$ git mv Lib/datetime.py Lib/_pydatetime.py 
$ git status
On branch clean_pydatetime
Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
	renamed:    Lib/datetime.py -> Lib/_pydatetime.py

Good, at this stage, Git understand that the file was renamed.

Create a new datetime.py file:

$ git show pydatetime:Lib/datetime.py > Lib/datetime.py
$ git add Lib/datetime.py
$ git status
On branch clean_pydatetime
Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
	new file:   Lib/_pydatetime.py
	modified:   Lib/datetime.py

Not good: _pydatetime.py is now seen as a "new file" :-(

Another test where the rename is a dedicated commit. Spoiler: not good, I get the same conflict on backport.

First commit, only to rename:

$ git switch -c clean_pydatetime  main
$ git mv Lib/datetime.py Lib/_pydatetime.py 
$ git ci -a -m "Rename Lib/datetime.py to Lib/_pydatetime.py"

Second commit, to recreate datetime.py:

$ git show pydatetime:Lib/datetime.py > Lib/datetime.py
$ git add Lib/datetime.py
$ git ci -a -m "Recreate Lib/datetime.py"

Now I created a bugfix (in the "main" branch);

$ git switch -c clean_pydatetime_bugfix
$ vim Lib/_pydatetime.py 
$ git ci -a -m "bugfix"
$ git show
 git show
commit f5e444261a1321b79bf27a4e0a05a422d6069c30 (HEAD -> clean_pydatetime_bugfix)
Author: Victor Stinner <vstinner@python.org>
Date:   Fri Nov 4 20:19:33 2022 +0100

    bugfix

diff --git a/Lib/_pydatetime.py b/Lib/_pydatetime.py
index 01742680a9..c22c6ccfb6 100644
--- a/Lib/_pydatetime.py
+++ b/Lib/_pydatetime.py
@@ -2371,7 +2371,7 @@ def tzname(self, dt):
         if isinstance(dt, datetime) or dt is None:
             if self._name is None:
                 return self._name_from_offset(self._offset)
-            return self._name
+            return self._name + "bugfix"
         raise TypeError("tzname() argument must be a datetime instance"
                         " or None")

What if I backport it to the 3.11 branch?

$ cd ../3.11
$ git switch -c clean_pydatetime_bugfix311
$ git cherry-pick -x clean_pydatetime_bugfix
CONFLICT (modify/delete): Lib/_pydatetime.py deleted in HEAD and modified in f5e444261a (bugfix).  Version f5e444261a (bugfix) of Lib/_pydatetime.py left in tree.
error: could not apply f5e444261a... bugfix
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

$ git status
On branch clean_pydatetime_bugfix311
You are currently cherry-picking commit f5e444261a.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	deleted by us:   Lib/_pydatetime.py

no changes added to commit (use "git add" and/or "git commit -a")

Well, I get the exact same conflict. git cherry-pick -x doesn't get that changes made in _pydatetime.py must be backported to datetime.py.

Note: git cherry-pick -x clean_pydatetime_bugfix is the same than git cherry-pick -x f5e444261a1321b79bf27a4e0a05a422d6069c30 and both commands only backport a single commit: the one changing a single line:

-            return self._name
+            return self._name + "bugfix"

For me, the quick conclusion is that we have 2 choices:

No change: keep 2642 lines of Python code in datetime.py
Create Lib/_pydatetime.py and backport manually changes from 3.12 to 3.11, and then rely on miss-ilington to automate the backport from 3.11 to 3.10

About manual backports, in 2 years (since 2021-01-01), Lib/datetime.py got 7 commits in the main branch. For comparison, it only got 2 commits in the 3.10 branch since 2021-01-01.

$ git log --after=2021-01-01 main Lib/datetime.py|grep ^commit|wc -l
7
$ git log --after=2021-01-01 3.10 Lib/datetime.py|grep ^commit|wc -l
2

IMO the low number of changes make it manageable to be done manually. What do you think?

Maybe a Git guru can help us here?

@pganssle

Create Lib/_pydatetime.py and backport manually changes from 3.12 to 3.11, and then rely on miss-ilington to automate the backport from 3.11 to 3.10

In clear, I'm in favor of this option.

@pganssle: Are you ok with that?

I think we should explore the option of a merge strategy that doesn't require manual backports, but if we can't find one, or if someone with git expertise can explain why there's no way to accomplish this without breaking backports, I'm fine with breaking backports.

I wonder if we could even special-case datetime in miss islington to preserve automated backports even without clean cherry picks.

I think we should explore the option of a merge strategy that doesn't require manual backports,

This issue is open for 2 years. I did many tests with Git.

So far, I didn't find anything obvious. Git doesn't give much freedom for that. Git doesn't store renames: it "recomputes" them in all operations, check if two files look similar or not.

I think we should explore the option of a merge strategy that doesn't require manual backports,

FWIW I did some additional tests, without finding an actual solution.

In the first test I tried to rename datetime.py to _pydatetime.py in a separate branch (let's call it B), then edit datetime.py in the previous branch (where it still existed -- let's call it A), and then merge B into A. When I did this, I found the edits I did in A to datetime.py ended up in _pydatetime.py.

Then I tried another strategy. I started from A and created a B branch where I renamed datetime.py to _pydatetime.py (just like above). Then I created a C branch from A where I edited datetime.py. By doing this git supposedly "knew" that in B, the content of _pydatetime.py came from datetime.py, and that in C we still had the same (now edited) datetime.py file (instead of having a new one (re)created from scratch). However when I tried to octopus merge B and C into A (with git merge B C while in A) I got some conflict. Swapping C and B also resulted in a conflict.

In the third and final attempt I was able to duplicate the file while preserving the history of both copies, by doing:

$ git switch -c B
Switched to a new branch 'B'
$ git mv Lib/datetime.py Lib/_pydatetime.py 
$ git commit -m 'Rename datetime.py to _pydatetime.py.'
[B a932519123] Rename datetime.py to _pydatetime.py.
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename Lib/{datetime.py => _pydatetime.py} (100%)
$ git show A:Lib/datetime.py > Lib/datetime.py
$ git add Lib/datetime.py
$ git commit -m 'Add back datetime.py.'
[B e1e1c92a5b] Add back datetime.py.
 1 file changed, 2642 insertions(+)
 create mode 100644 Lib/datetime.py
$ git switch A 
Switched to branch 'A'
$ git merge --no-ff B  # apparently the --no-ff is important here
Merge made by the 'ort' strategy.
 Lib/_pydatetime.py | 2642 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 2642 insertions(+)
 create mode 100644 Lib/_pydatetime.py
$ vim Lib/datetime.py  # edit/remove stuff
$ git add Lib/datetime.py 
$ git commit -m 'Update datetime.py'
[A 1d46a8cd30] Update datetime.py
 1 file changed, 21 insertions(+), 2642 deletions(-)
 rewrite Lib/datetime.py (99%)

Now _pydatetime.py contains the content of the original datetime.py (with all the history) and datetime.py contains the updated content (also with the history of whatever is left in there) and git should "know" that both files come from the original datetime.py. However I'm not sure if git is able to figure out how to backport changes correctly and I ran out of time before being able to test backports.

@ezio-melotti

I think we should explore the option of a merge strategy that doesn't require manual backports, but if we can't find one, or if someone with git expertise can explain why there's no way to accomplish this without breaking backports, I'm fine with breaking backports.

This issue is open for 2 years. @ezio-melotti and me investigated options for backports and no solution was found.

@pganssle: So are you ok to create Lib/_pydateime.py to make "import datetime" faster?

I have no idea of miss-islington. The bot just runs git cherry-pick -x. I don't know how to customize this command.

This issue is open for 2 years.

Sounds like it's not very urgent, then.

Seriously, Victor, I appreciate the work you've put in on this, but I don't appreciate the rush you are imparting here. I didn't ask you to revive your PR when you closed the issue, I just said that it's still an open issue. I think it is worth taking a little bit of time investigating, but no one has provided a compelling explanation as to why this cannot work, nor have I had time to investigate this sufficiently to rule out a solution that doesn't require several years of manual backports. The feature freeze isn't for ~half a year.

@pganssle

What I mean is that people had 2 years to look into the Git issue and so far nobody was able to provide a solution to the Git issue. So for me, there is no solution, and there are only 2 choices: do nothing (keep datetime.py as it is), or create Lib/_pydatetime.py and handle backports manually.

You need more time, ok. In this case, I abandon my PR and I leave the issue for someone else. I consider that @pganssle is the datetime maintainer, and I prefer to have his bless for this change. But I don't have the bandwidth to spend more time on this topic (investigate the Git question). I care about "import datetime" performance, but not enough to spend more time. For me, I don't think that it's worth it.

Anyone is free to clone my PR #99090 and restart from that. By the way, there is now a conflict, I guess that datetime.py was modified since I created my PR. That's one of the reason why I don't like to keep a PR open for too long. The other reason is that PR that I created are my TODO list and I want to keep this list as short as possible. Otherwise, I lost track.

I closed this issue 1 month ago because the issues that I created are my second "TODO list" (and I want to keep it short). Here it seems like the consensus is towards "do nothing". But well, I will not close again the issue. I will just learn to skip it from my "TODO list" :-) Or maybe there is a way in GitHub to transfer the "ownership" of an issue to someone else. I don't know. I don't care much in fact :-)

pythonGH-84976: Move Lib/datetime.py to Lib/_pydatetime

pythonGH-84976: Re-introduce `datetime.py` and fix reprs

pythonGH-84976: Move Lib/datetime.py to Lib/_pydatetime

pythonGH-84976: Re-introduce `datetime.py` and fix reprs

GH-84976: Move Lib/datetime.py to Lib/_pydatetime

GH-84976: Re-introduce `datetime.py` and fix reprs

gh-84976: Add back UTC to datetime.__all__ (#104920)

pythongh-84976: Add back UTC to datetime.__all__ (pythonGH-104920)

[3.12] gh-84976: Add back UTC to datetime.__all__ (GH-104920) (#106019)

Navigation Menu

Repository navigation

Create Lib/_pydatetime.py file to optimize "import datetime" when _datetime is available #84976

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Create Lib/_pydatetime.py file to optimize "import datetime" when _datetime is available #84976

Description

Activity

vstinner commented on Nov 3, 2022

pganssle commented on Nov 4, 2022

vstinner commented on Nov 4, 2022

serhiy-storchaka commented on Nov 4, 2022

vstinner commented on Nov 4, 2022

vstinner commented on Nov 4, 2022

pganssle commented on Nov 4, 2022

vstinner commented on Nov 4, 2022

vstinner commented on Nov 4, 2022

vstinner commented on Nov 4, 2022

vstinner commented on Nov 4, 2022

vstinner commented on Nov 9, 2022

pganssle commented on Nov 10, 2022

vstinner commented on Nov 10, 2022

ezio-melotti commented on Nov 11, 2022

vstinner commented on Nov 28, 2022

pganssle commented on Nov 28, 2022

vstinner commented on Nov 29, 2022

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions