Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-45953: Statically allocate the main interpreter (and initial thread state). #29883

Merged
merged 78 commits into from
Jan 12, 2022

Conversation

ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented Dec 1, 2021

Currently the main interpreter is allocated on the heap during runtime initialization. Here we are instead embedding it into _PyRuntimeState, which means it is statically allocated as part of the _PyRuntime global. The same goes for the initial thread state (of each interpreter, including the main one). Consequently there are fewer allocations during runtime/interpreter init, fewer possible failures, and better memory locality.

FYI, this also helps efforts to consolidate globals, which in turns helps work on subinterpreter isolation.

https://bugs.python.org/issue45953

Python/ceval.c Outdated
int
_PyEval_InitState(struct _ceval_state *ceval)
void
_PyEval_InitState(struct _ceval_state *ceval, PyThread_type_lock pending_lock)
Copy link
Member

Choose a reason for hiding this comment

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

Would this be a good opportunity to remove this function?
The _pending_calls struct belongs on the interpreter state, not on the ceval as only the main interpreter can handle them.

Copy link
Member Author

Choose a reason for hiding this comment

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

There's enough going on in this PR that I'd rather not.

Modules/posixmodule.c Outdated Show resolved Hide resolved
Modules/posixmodule.c Outdated Show resolved Hide resolved
Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

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

A couple of tests have been inverted.

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

Include/cpython/pystate.h Outdated Show resolved Hide resolved
Include/internal/pycore_interp.h Outdated Show resolved Hide resolved
};

#define _PyInterpreterState_INIT \
Copy link
Member

Choose a reason for hiding this comment

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

This is only used once, please remove it.

Copy link
Member Author

Choose a reason for hiding this comment

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

FYI, I brought this back to avoid so much nesting and clutter in _PyRuntimeState_INIT.

Include/internal/pycore_runtime.h Outdated Show resolved Hide resolved
Include/internal/pycore_runtime.h Outdated Show resolved Hide resolved
// because on Windows we only get a pointer type.
// All other pointers in PyThreadState are either
// populated lazily or change but default to NULL.
} _preallocated;
} _PyRuntimeState;

#define _PyRuntimeState_INIT \
Copy link
Member

Choose a reason for hiding this comment

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

Might as well remove this as well. It is also only used once.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should _Py_global_objects_INIT also move? It's pretty big and likely to get much bigger.

Copy link
Member Author

@ericsnowcurrently ericsnowcurrently Jan 10, 2022

Choose a reason for hiding this comment

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

Actually, _PyRuntimeState_INIT is used twice.

This comment was marked as off-topic.

@ericsnowcurrently
Copy link
Member Author

With basically all feedback addressed I plan on merging this soon, to unblock other changes I have waiting. If I missed something, I'd be glad to address it in a follow-up PR.

@gvanrossum
Copy link
Member

gvanrossum commented Jan 12, 2022 via email

@ericsnowcurrently ericsnowcurrently merged commit ed57b36 into python:main Jan 12, 2022
@ericsnowcurrently ericsnowcurrently deleted the preallocate-main branch January 12, 2022 23:28
tacaswell added a commit to tacaswell/typed_ast that referenced this pull request Jan 25, 2022
As of CPython 3.11 (via python/cpython#29883) stdbool.h
is now included in Python.h so do attempt to redefine bool/true/false.
srittau pushed a commit to python/typed_ast that referenced this pull request Jan 25, 2022
As of CPython 3.11 (via python/cpython#29883) stdbool.h
is now included in Python.h so do attempt to redefine bool/true/false.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants