Uh oh!
There was an error while loading. Please reload this page.
gh-88473: Implement fast path in date.today() for date types - #130980
Conversation
picnixz
commented
Mar 8, 2025
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
Both are just plain builds. python3.14 is from the morning but I doubt the few commits matter. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
picnixz
commented
Mar 8, 2025
By plain builds, do you mean with or withou |
StanFromIreland
commented
Mar 9, 2025
@pganssle can you take a look at this? |
Uh oh!
There was an error while loading. Please reload this page.
picnixz
left a comment
There was a problem hiding this comment.
On my part, it looks good but I'll let a datetime expert have the final decision.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
StanFromIreland
commented
May 5, 2025
Friendly ping @pganssle Any comments on this? |
StanFromIreland
commented
Jun 25, 2025
Friendly ping @pganssle :-) |
pganssle
left a comment
There was a problem hiding this comment.
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:
staticPyObject*date_today(PyObject*cls, PyObject*Py_UNUSED(dummy))
{
/* Use C implementation to boost performance for date type */structtmtm;
time_tt;
time(&t);
if (_PyTime_localtime(t, &tm) !=0) {
returnNULL;
}
returnnew_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.
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 |
# Conflicts: # Modules/_datetimemodule.c
StanFromIreland
commented
Sep 15, 2025
It will be incorrect though? Since |
pganssle
commented
Sep 16, 2025
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 |
Other suggested implementations are not as backward compatible.
~5x faster for date types