Skip to content

gh-118527: Intern code consts in free-threaded build - #118667

Merged
colesbury merged 8 commits into
python:mainfrom
colesbury:gh-118527-code-consts
May 7, 2024
Merged

gh-118527: Intern code consts in free-threaded build#118667
colesbury merged 8 commits into
python:mainfrom
colesbury:gh-118527-code-consts

Conversation

@colesbury

@colesburycolesbury commented May 6, 2024

Copy link
Copy Markdown
Contributor

We already intern and immortalize most string constants. In the free-threaded build, other constants are a frequent source of reference count contention because they are shared by all threads running the same code objects.

For example, the following function would not otherwise scale because 1.5 would be a non-immortalized constant shared by all threads that execute run_in_parallel.

defrun_in_parallel(x, iters):
for_inrange(iters):
x*=1.5returnx

We already intern and immortalize most string constants. In the
free-threaded build, other constants can be a source of reference count
contention because they are shared by all threads running the same code
objects.
@colesbury

Copy link
Copy Markdown
ContributorAuthor

!buildbot nogil

@bedevere-bot

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @colesbury for commit 5e8c0f0 🤖

The command will test the builders whose names match following regular expression: nogil

The builders matched are:

  • x86-64 MacOS Intel ASAN NoGIL PR
  • AMD64 Ubuntu NoGIL Refleaks PR
  • ARM64 MacOS M1 NoGIL PR
  • AMD64 Windows Server 2022 NoGIL PR
  • AMD64 Ubuntu NoGIL PR
  • ARM64 MacOS M1 Refleaks NoGIL PR
  • x86-64 MacOS Intel NoGIL PR

Comment threadObjects/codeobject.c
Comment threadInclude/internal/pycore_code.h
Comment threadObjects/codeobject.c
else if (PyTuple_CheckExact(op)) {
Py_ssize_t size = PyTuple_GET_SIZE(op);
PyObject **data = _PyTuple_ITEMS(op);
return _Py_HashBytes(data, sizeof(PyObject *) * size);

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

We first intern and de-dupe the contents so identity comparisons (including hasing pointers) is sufficient.

For example the following prints True with this PR:

deffoo():
return (0.0, (1, 2, "hello"))
defbar():
return (0.0, (1, 2, "hello"))
print(foo() isbar())

@DinoVDinoV left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Adding the mutex to LOCKS_INIT is really the most important thing, the slices are probably fine either way, it'd just be a little less duplication if we followed them, but they're probably usually in the small ints list anyway.

@rhettinger
rhettinger removed their request for review May 6, 2024 19:36
@colesbury
colesbury requested a review from DinoVMay 6, 2024 20:53
@colesbury

Copy link
Copy Markdown
ContributorAuthor

!buildbot nogil

@bedevere-bot

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @colesbury for commit f5cab78 🤖

The command will test the builders whose names match following regular expression: nogil

The builders matched are:

  • x86-64 MacOS Intel ASAN NoGIL PR
  • AMD64 Ubuntu NoGIL Refleaks PR
  • ARM64 MacOS M1 NoGIL PR
  • AMD64 Windows Server 2022 NoGIL PR
  • AMD64 Ubuntu NoGIL PR
  • ARM64 MacOS M1 Refleaks NoGIL PR
  • x86-64 MacOS Intel NoGIL PR

@colesbury
colesbury merged commit 723d4d2 into python:mainMay 7, 2024
@colesbury
colesbury deleted the gh-118527-code-consts branch May 7, 2024 00:12
SonicField pushed a commit to SonicField/cpython that referenced this pull request May 8, 2024
…8667)
We already intern and immortalize most string constants. In the
free-threaded build, other constants can be a source of reference count
contention because they are shared by all threads running the same code
objects.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@colesbury@bedevere-bot@DinoV