◐ Shell
reader mode source ↗
Skip to content

gh-71587: Isolate _datetime#102995

Closed
erlend-aasland wants to merge 33 commits into
python:mainfrom
erlend-aasland:isolate-datetime/poc
Closed

gh-71587: Isolate _datetime#102995
erlend-aasland wants to merge 33 commits into
python:mainfrom
erlend-aasland:isolate-datetime/poc

Conversation

@erlend-aasland

@erlend-aasland erlend-aasland commented Mar 24, 2023 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

@erlend-aasland
Consistency: use explicit initialiser for m_base
00c0ef0
@erlend-aasland
Add module state stub; establish global state on stack
d1bbebb
@erlend-aasland
Put conversion factors in state struct
efeea77
@erlend-aasland
Move PyDateTime_TimeZone_UTC to state
ef4a4c2
@erlend-aasland
Move PyDateTime_Epoch to state struct
9e3f693
@erlend-aasland
Remove datetime_strptime globals entry
5bd60b2
@erlend-aasland
Add heap type machinery
b5c051b
@erlend-aasland
PyDateTime_DateTimeType
0b23934
@erlend-aasland
PyDateTime_DateType
9184c37
@erlend-aasland
PyDateTime_DeltaType
d890013
@erlend-aasland
PyDateTime_IsoCalendarDateType
780f261
@erlend-aasland
Regen clinic
2d16589
@erlend-aasland
PyDateTime_TZInfoType and PyDateTime_TimeZoneType
5566ce5
@erlend-aasland
PyDateTime_TimeType
89775f3
@erlend-aasland
Visit and clear all types
e798939
@erlend-aasland
Get rid of GLOBAL_STATE
78e0023
@erlend-aasland
Fix binary ops
d392c1a
@erlend-aasland
Unexpose IsoCalendarType
7aaf57f
@erlend-aasland
_PyType_GetModuleState
206cdcc
@erlend-aasland
Pull in main
304ca28
@erlend-aasland
Remove time/datetime memory optimisation
8c53fa1
@erlend-aasland
Remove unused get_module_state_by_cls() helper
b583954
@erlend-aasland erlend-aasland changed the title [PoC] Isolate _datetime Mar 24, 2023
@bedevere-bot bedevere-bot mentioned this pull request Mar 24, 2023
Closed
@erlend-aasland

erlend-aasland commented Mar 24, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Observations:

  • the (fishy) time and datetime memory optimisations may need to go; for now, I tore them out
  • it may be beneficial to store state in object context instead of passing it around
  • the exposed C API is still using a global variable; this is unfortunate

@erlend-aasland

erlend-aasland commented Mar 29, 2023

Copy link
Copy Markdown
Contributor Author

@ericsnowcurrently, regarding the datetime C API:

Currently, the encapsulated datetime C API is exposed as a global variable:

cpython/Include/datetime.h

Lines 196 to 197 in e375bff

/* Define global variable for the C API and a macro for setting it. */
static PyDateTime_CAPI *PyDateTimeAPI = NULL;

I guess we could move this to the interpreter state instead. Thoughts?

FTR, if we run the ref leak bots on this PR, they fail because of the datetime C API tests in testcapi:

cpython/Modules/_testcapi/datetime.c

Lines 8 to 29 in e375bff

static PyObject *
test_datetime_capi(PyObject *self, PyObject *args)
{
if (PyDateTimeAPI) {
if (test_run_counter) {
/* Probably regrtest.py -R */
Py_RETURN_NONE;
}
else {
PyErr_SetString(PyExc_AssertionError,
"PyDateTime_CAPI somehow initialized");
return NULL;
}
}
test_run_counter++;
PyDateTime_IMPORT;
if (PyDateTimeAPI) {
Py_RETURN_NONE;
}
return NULL;
}

@ericsnowcurrently ericsnowcurrently mentioned this pull request Mar 29, 2023
Closed
25 tasks
vstinner
vstinner reviewed Jul 12, 2023
View reviewed changes

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

Hide comment

Would it be possible to split the PR into multiple parts? Example:

  • PR 1: Add a module state, refer a few static types there, and pass the state to the easy places to retrieve the type
  • PR 2: Slowly, convert static types, one by one
  • PR 3: Dirty changes
  • PR 4: The final beautiful change which just remove the old code

@erlend-aasland

erlend-aasland commented Jul 12, 2023

Copy link
Copy Markdown
Contributor Author

Would it be possible to split the PR into multiple parts? Example:

  • PR 1: Add a module state, refer a few static types there, and pass the state to the easy places to retrieve the type
  • PR 2: Slowly, convert static types, one by one
  • PR 3: Dirty changes
  • PR 4: The final beautiful change which just remove the old code

Definitely!

This was referenced Oct 6, 2023
Closed
Closed
erlend-aasland added a commit to erlend-aasland/cpython that referenced this pull request Oct 6, 2023
@erlend-aasland
Apply Victor's suggestions from pythongh-102995
2eb3315
@Eclips4 Eclips4 mentioned this pull request Nov 15, 2023
Closed
This was referenced Mar 31, 2024
Closed
Closed
@erlend-aasland erlend-aasland deleted the isolate-datetime/poc branch June 2, 2024 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner left review comments

@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303

@pganssle pganssle Awaiting requested review from pganssle pganssle is a code owner

@StanFromIreland StanFromIreland Awaiting requested review from StanFromIreland StanFromIreland will be requested when the pull request is marked ready for review StanFromIreland is a code owner

@ericsnowcurrently ericsnowcurrently ericsnowcurrently will be requested when the pull request is marked ready for review ericsnowcurrently is a code owner

Assignees

No one assigned

Labels

awaiting core review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@erlend-aasland @vstinner @bedevere-bot