FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-88473: Implement fast path in date.today() for date types by StanFromIreland · Pull Request #130980 · python/cpython · GitHub

/ cpython Public

gh-88473: Implement fast path in date.today() for date types - #130980

Merged
pganssle merged 8 commits into
python:mainfrom
StanFromIreland:date_speedup
Sep 16, 2025
ConversationCommits (8)ChecksFiles changed
Merged

gh-88473: Implement fast path in date.today() for date types#130980
pganssle merged 8 commits into
python:mainfrom
StanFromIreland:date_speedup

Conversation

StanFromIreland commented Mar 8, 2025 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

Other suggested implementations are not as backward compatible.

$ ./python -m timeit -s 'from datetime import date' 'date.today()'
1000000 loops, best of 5: 271 nsec per loop
$ python3.14 -m timeit -s 'from datetime import date' 'date.today()'
200000 loops, best of 5: 1.36 usec per loop

~5x faster for date types

bedevere-app Bot mentioned this pull request Mar 8, 2025
Closed

picnixz commented Mar 8, 2025

Copy link
Copy Markdown
Member

Are the benchmarks realised on a PGO build or a debug build? or maybe PGO+LTO? or just release build?

StanFromIreland commented Mar 8, 2025

Copy link
Copy Markdown
Member Author

Both are just plain builds. python3.14 is from the morning but I doubt the few commits matter.

picnixz reviewed Mar 8, 2025
View reviewed changes
Comment thread Modules/_datetimemodule.c
Comment thread Modules/_datetimemodule.c
Comment thread Modules/_datetimemodule.c Outdated
Comment thread Modules/_datetimemodule.c Outdated
Comment thread Modules/_datetimemodule.c Outdated
Comment thread Modules/_datetimemodule.c Outdated
Comment thread Misc/NEWS.d/next/Library/2025-03-08-17-07-00.gh-issue-88473.qg23g8.rst Outdated

picnixz commented Mar 8, 2025

Copy link
Copy Markdown
Member

By plain builds, do you mean with or withou --with-pydebug? I assume without so it's probably release builds so it should be fine. But please the configure flags

StanFromIreland requested a review from picnixz March 8, 2025 22:06

StanFromIreland commented Mar 9, 2025

Copy link
Copy Markdown
Member Author

@pganssle can you take a look at this?

picnixz reviewed Mar 12, 2025
View reviewed changes
Comment thread Modules/_datetimemodule.c Outdated
StanFromIreland requested a review from picnixz March 12, 2025 18:27
picnixz approved these changes Mar 12, 2025
View reviewed changes

picnixz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

On my part, it looks good but I'll let a datetime expert have the final decision.

Comment thread Modules/_datetimemodule.c
Comment thread Modules/_datetimemodule.c Outdated
Benedikt's final nits ;-)
372a764 
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

StanFromIreland commented May 5, 2025

Copy link
Copy Markdown
Member Author

Friendly ping @pganssle Any comments on this?

StanFromIreland commented Jun 25, 2025

Copy link
Copy Markdown
Member Author

Friendly ping @pganssle :-)

pganssle requested changes Sep 15, 2025
View reviewed changes

pganssle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This is an acceptable way to do it, but IIRC we already pushed some of this kind of dispatching logic in new_date_subclass_ex, so I think you can replace the entire contents of date_today with this:

static PyObject *
date_today(PyObject *cls, PyObject *Py_UNUSED(dummy))
{
    /* Use C implementation to boost performance for date type */
    struct tm tm;
    time_t t;
    time(&t);

    if (_PyTime_localtime(t, &tm) != 0) {
        return NULL;
    }

    return new_date_subclass_ex(tm.tm_year + 1900,
                       tm.tm_mon + 1,
                       tm.tm_mday,
                       (PyObject *)cls);
}

And you'll get the same speedup and as a bonus datetime.today() will also get the same speedup.

bedevere-app Bot commented Sep 15, 2025

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Merge branch 'refs/heads/main' into date_speedup
3bfd3ff 
# Conflicts:
#	Modules/_datetimemodule.c

StanFromIreland commented Sep 15, 2025

Copy link
Copy Markdown
Member Author

And you'll get the same speedup and as a bonus datetime.today() will also get the same speedup.

It will be incorrect though? Since datetime.today has hours, minutes etc.

pganssle commented Sep 16, 2025

Copy link
Copy Markdown
Member

It will be incorrect though? Since datetime.today has hours, minutes etc.

Hah, oops, right, right. Should have run the test suite and not just the benchmark.

OK, let's go with this as it is and if we want to speed up datetime.today we can add an implementation for datetime.datetime that is just effectively return datetime.now(tz=None).

pganssle merged commit fc3e22a into python:main Sep 16, 2025
48 of 49 checks passed
maurycy mentioned this pull request May 14, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

pganssle pganssle approved these changes
picnixz picnixz approved these changes
abalkin Awaiting requested review from abalkin

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL