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

The Limit of 32 Pending Calls is Too Small #110693

Open
ericsnowcurrently opened this issue Oct 11, 2023 · 3 comments
Open

The Limit of 32 Pending Calls is Too Small #110693

ericsnowcurrently opened this issue Oct 11, 2023 · 3 comments
Assignees
Labels
3.13 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-subinterpreters type-feature A feature request or enhancement

Comments

@ericsnowcurrently
Copy link
Member

ericsnowcurrently commented Oct 11, 2023

We currently have a limit of 32 pending calls for each interpreter. This has been the limit pretty much since pending calls were added (originally for signal handling). Now that we use pending calls between interpreters (e.g. to decref an object under the correct interpreter), a maximum of 32 is too low. This will become a bigger problem as subinterpreters are used more.

The current workaround looks something like this:

    while (_PyEval_AddPendingCall(interp, func, arg, flags) < 0) {
        // Keep trying until the pending calls is added.
    };

...perhaps also involving a separate queue or an extra thread.

Possible improvements:

  • substantially increase the limit, e.g. to 500 (and keep using the statically allocated array)
  • use a linked list with no limit
  • use a linked list with some large limit (to prevent accidental out-of-control growth)
  • use a linked list but "allocate" from a static array for the first 32 pending calls

(This issue is a continuation of gh-79647.)

Linked PRs

@ericsnowcurrently ericsnowcurrently added type-feature A feature request or enhancement interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-subinterpreters 3.13 bugs and security fixes labels Oct 11, 2023
@ericsnowcurrently ericsnowcurrently self-assigned this Oct 11, 2023
@kumaraditya303
Copy link
Contributor

I would go with a linked list implementation with no limits. I don't think limiting it has any benefit.

@ericsnowcurrently
Copy link
Member Author

ericsnowcurrently commented Oct 13, 2023

For 3.12 the simplest thing would be to bump NPENDINGCALLS from 32 to something really large (e.g. 1000), though that is a bit wasteful of memory. Backporting the linked list implementation might be okay too.

CC @Yhg1s

@gvanrossum
Copy link
Member

(Maybe change the title to "... is Too Small"? "Few" sounds odd here. Also maybe don't use Title Caps. :-)

@ericsnowcurrently ericsnowcurrently changed the title The Limit of 32 Pending Calls is Too Few The Limit of 32 Pending Calls is Too Small Mar 6, 2024
ericsnowcurrently added a commit that referenced this issue Apr 26, 2024
This does some cleanup in preparation for later changes.
ericsnowcurrently added a commit that referenced this issue Apr 27, 2024
…18302)

This is an improvement over the status quo, reducing the likelihood of completely filling the pending calls queue.  However, the problem won't go away completely unless we move to an unbounded linked list or add a mechanism for waiting until the queue isn't full.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-subinterpreters type-feature A feature request or enhancement
Projects
Status: Todo
Development

No branches or pull requests

3 participants