Skip to content

Concurrent reads from cell objects (LOAD_DEREF) do not scale well #123358

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.

Sign up for GitHub

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

Jump to bottom
Closed
colesbury opened this issue Aug 26, 2024 · 2 comments
Closed

Concurrent reads from cell objects (LOAD_DEREF) do not scale well #123358

colesbury opened this issue Aug 26, 2024 · 2 comments
Assignees
@corona10
Labels
performance Performance or resource usage sprint topic-free-threading

Comments

@colesbury
Copy link
Contributor

colesbury commented Aug 26, 2024 •
edited by bedevere-app bot
Loading

Concurrent accesses from multiple threads to the same cell object do not scale well in the free-threaded build. There are two problems:

  1. LOAD_DEREF calls PyCell_GetRef, which acquires the per-object lock on the cell instance. We'll need to use _Py_TryXGetRef or similar to safely access the value.

  2. PyCell_GetRef returns a strong reference and LOAD_DEREF will need a _PyStackRef to handle objects that use deferred reference counting.

Linked PRs
@colesbury colesbury added performance Performance or resource usage topic-free-threading labels Aug 26, 2024
@colesbury
Copy link
Contributor Author

This came up in the context of wjakob/nanobind#695

@corona10
Copy link
Member

@colesbury Can I take a look at it?

@corona10 corona10 self-assigned this Sep 24, 2024
corona10 added a commit to corona10/cpython that referenced this issue Sep 24, 2024
@corona10
pythongh-123358: Update LOAD_DEREF to use stackref and atomic incref
beaa325
@bedevere-app bedevere-app bot mentioned this issue Sep 24, 2024
Closed
corona10 added a commit to corona10/cpython that referenced this issue Sep 25, 2024
@corona10
pythongh-123358: Update LOAD_DEREF to use stackref and atomic incref
23776cf
corona10 added a commit to corona10/cpython that referenced this issue Sep 25, 2024
@corona10
pythongh-123358: Update LOAD_DEREF to use stackref and atomic incref
9fe9706
corona10 added a commit to corona10/cpython that referenced this issue Sep 25, 2024
@corona10
pythongh-123358: Update LOAD_DEREF to use stackref and atomic incref
d9e018c
corona10 added a commit to corona10/cpython that referenced this issue Oct 3, 2024
@corona10
pythongh-123358: Update LOAD_DEREF to use stackref and atomic incref
b834dc5
colesbury pushed a commit to colesbury/cpython that referenced this issue Feb 13, 2025
@corona10 @colesbury
pythongh-123358: Update LOAD_DEREF to use stackref and atomic incref
57526e8
@bedevere-app bedevere-app bot mentioned this issue Feb 13, 2025
Merged
colesbury added a commit to colesbury/cpython that referenced this issue Mar 24, 2025
@colesbury
Merge branch 'main' into pythongh-123358-load-deref
70a09df
colesbury added a commit that referenced this issue Mar 26, 2025
@colesbury @corona10
gh-123358: Use _PyStackRef in LOAD_DEREF (gh-130064)
3d4ac1a 
Concurrent accesses from multiple threads to the same `cell` object did not
scale well in the free-threaded build. Use `_PyStackRef` and optimistically
avoid locking to improve scaling.

With the locks around cell reads gone, some of the free threading tests were
prone to starvation: the readers were able to run in a tight loop and the
writer threads weren't scheduled frequently enough to make timely progress.
Adjust the tests to avoid this.

Co-authored-by: Donghee Na <[email protected]>
@github-project-automation github-project-automation bot moved this from Todo to Done in Sprint 2024 Mar 26, 2025
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 1, 2025
@colesbury @corona10 @diegorusso
pythongh-123358: Use _PyStackRef in LOAD_DEREF (pythongh-130064)
37ab996 
Concurrent accesses from multiple threads to the same `cell` object did not
scale well in the free-threaded build. Use `_PyStackRef` and optimistically
avoid locking to improve scaling.

With the locks around cell reads gone, some of the free threading tests were
prone to starvation: the readers were able to run in a tight loop and the
writer threads weren't scheduled frequently enough to make timely progress.
Adjust the tests to avoid this.

Co-authored-by: Donghee Na <[email protected]>
seehwan pushed a commit to seehwan/cpython that referenced this issue Apr 16, 2025
@colesbury @corona10 @seehwan80
pythongh-123358: Use _PyStackRef in LOAD_DEREF (pythongh-130064)
317e536 
Concurrent accesses from multiple threads to the same `cell` object did not
scale well in the free-threaded build. Use `_PyStackRef` and optimistically
avoid locking to improve scaling.

With the locks around cell reads gone, some of the free threading tests were
prone to starvation: the readers were able to run in a tight loop and the
writer threads weren't scheduled frequently enough to make timely progress.
Adjust the tests to avoid this.

Co-authored-by: Donghee Na <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@corona10 corona10

Labels
performance Performance or resource usage sprint topic-free-threading
Projects
Sprint 2024
Archived in project
Milestone
No milestone
Development

No branches or pull requests
5 participants
@colesbury @corona10 and others