Repository navigation
PEP 3121, 384 refactoring applied to _datetime module #59595
Description
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 2012-07-18.21:33:52.042>
labels = ['extension-modules', '3.11', 'performance']
title = 'PEP 3121, 384 refactoring applied to _datetime module'
updated_at = <Date 2021-12-08.15:27:50.710>
user = 'https://bugs.python.org/RobinSchreiber'bugs.python.org fields:
activity = <Date 2021-12-08.15:27:50.710>
actor = 'iritkatriel'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Extension Modules']
creation = <Date 2012-07-18.21:33:52.042>
creator = 'Robin.Schreiber'
dependencies = []
files = ['26431', '26803', '26817', '28274']
hgrepos = []
issue_num = 15390
keywords = ['patch', 'pep3121']
message_count = 12.0
messages = ['165805', '166126', '168215', '168218', '168229', '168266', '168334', '170154', '170249', '177274', '177277', '408025']
nosy_count = 8.0
nosy_names = ['lemburg', 'loewis', 'belopolsky', 'pitrou', 'asvetlov', 'Robin.Schreiber', 'p-ganssle', 'iritkatriel']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue15390'
versions = ['Python 3.11']
Activity
RobinSchreiber commentedon Jul 18, 2012
Changes proposed in PEP-3121 and PEP-384 have now been applied to the datetime module!
asvetlov commentedon Jul 22, 2012
Too late for 3.3
RobinSchreiber commentedon Aug 14, 2012
Fixed _dealloc methods. Also: Init now returns the previously initialized module if available.
asvetlov commentedon Aug 14, 2012
Add Alexander Belopolsky to nosy list as maintainer of datetime module.
pitrou commentedon Aug 14, 2012
+ Py_CLEAR(_datetimemodulestate(m)->PyDateTime_DateTimeType);
+ Py_CLEAR(_datetimemodulestate(m)->PyDateTime_DeltaType);
+ Py_CLEAR(_datetimemodulestate(m)->PyDateTime_TimeType);
+ Py_CLEAR(_datetimemodulestate(m)->PyDateTime_TimeZoneType);
+ Py_CLEAR(_datetimemodulestate(m)->PyDateTime_DateType);
+ Py_CLEAR(_datetimemodulestate(m)->PyDateTime_TZInfoType);
Style nit: I would really store the module state pointer in a variable here, instead of repeating _datetimemodulestate(m) every line.
(same in other places, such as the module init function)
+PyObject* _Get_State(struct PyModuleDef*);
I'm not sure why a module should define such generic a function, and especially not without a "Py" prefix.
Besides, review comments from bpo-15653 apply here.
RobinSchreiber commentedon Aug 15, 2012
I have now included the changes that Antoine suggested. The _Get_State was used for debugging purposes and is, as I think, no longer necessary.
However we have yet to find a solution for the decref inside the dealloc methods.
pitrou commentedon Aug 15, 2012
Perhaps ask for advice on python-dev?
abalkin commentedon Sep 10, 2012
For some reason there are no review links, so I'll review in this message.
Include/datetime.h
+typedef struct {
..
+} _datetimemodulestate;
Names exposed in public headers (datetime.h is a public header) should start with Py or _Py. Other offenders include _datetimemodule_state, _datetimemodule, _datetimemodulestate_global.
I don't think these names need to be defined in the header file at all.
+typedef struct {
+ /* Forward declarations. */
+ PyObject *PyDateTime_DateType;
+ PyObject *PyDateTime_DateTimeType;
...
These are not forward declarations anymore. There is no need for PyDateTime_ prefix. Use names from PyDateTime_CAPI struct.
abalkin commentedon Sep 10, 2012
I would like to split this issue to separate PEP-3121 changes from PEP-384. PEP-3121 state cleanup implementation is clearly an improvement "from a resource management point of view." On the other hand, I don't see much benefit for the datetime module from using a stable ABI. Unless I am missing something, PEP-384 is primarily benefiting 3rd party developers who distribute binary modules that should run under multiple Python versions. ABI stability is not a concern for the stdlib modules.
On the other hand, the price for multi-version support is rather steep. Statically allocated types are more efficient. For example, with static types, PyDate_CheckExact() is a simple macro that expands into a dereference and a pointer comparison - a couple of instructions at the CPU level. On the other hand, with a proposed patch, it will involve a function call to locate the module (PyState_FindModule), followed by another function call to locate the state (PyModule_GetState) and several more dereferences that may lead to cache misses and other pessimizations.
There is an important behavior change related to multiple interpreters. Currently dates created by different interpreters have the same type. With the proposed change they will have different types. I don't think this is desirable.
In short, let's go in baby steps. Let's implement PEP-3121 cleanup first and start a debate on the role of PEP-384 in stdlib.
RobinSchreiber commentedon Dec 10, 2012
I have updated the patch to work again with the current version of the _datetimemodule.
Regarding the suggestion of separating PEP-3121 and PEP-384. It might be true that datetime and other modules do not benefit directly from PEP-384, however it is still a fact that the stdlib modules should be seen as a set of reference modules, that are all implemented in a way that complies with the implementation fo the xxmodules.
I have talked with Martin von Löwis about this, and as far as I understood him correctly he also sees the PEP-384 refactoring applied to the whole stdlib as a nessecary "signal" to other developers to refactor their modules accordingly.
Anyway I am planning to start to commit all of the open changes that I have created during my GSOC in the next few months. So a decision regarding this separation concern might be helpful. :-)
malemburg commentedon Dec 10, 2012
On 10.12.2012 11:39, Robin Schreiber wrote:
Please use "_Py_" prefixes for private symbols you put in the header
files, e.g. _datetimemodulestate and the macros.
Question: What happens if PyModule_GetState() or PyState_FindModule()
raise an exception and return NULL ?
The current code will segfault in such a situation.
Thanks,
Marc-Andre Lemburg
eGenix.com
[-]PEP 3121, 384 refactoring applied to datetime module[/-][+]PEP 3121, 384 refactoring applied to _datetime module[/+]iritkatriel commentedon Dec 8, 2021
The patch needs to be converted to a github PR.
iritkatriel commentedon Sep 11, 2022
@robinschreiber I'm closing this because, a decade on, the patch is very much out of date. Please create a new issue if you would like to pursue this.