gh-105699: Add Some Stress Tests for Subinterpreter Creation by ericsnowcurrently · Pull Request #106966 · python/cpython · GitHub
Skip to content

gh-105699: Add Some Stress Tests for Subinterpreter Creation - #106966

Merged
ambv merged 1 commit into
python:mainfrom
ericsnowcurrently:interpreter-stress-tests
Jul 22, 2023
Merged

gh-105699: Add Some Stress Tests for Subinterpreter Creation#106966
ambv merged 1 commit into
python:mainfrom
ericsnowcurrently:interpreter-stress-tests

Conversation

@ericsnowcurrently

Copy link
Copy Markdown
Member

The tests would have caught the crashes from gh-105699.

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.

Suggested change
def test_create_many_threaded(self):
@requires_resource('cpu')
def test_create_many_threaded(self):

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hmm, what is the effect of this decorator? Also, what is it telling readers?

I wouldn't consider this test to be particularly CPU-intensive. We do spin up a couple hundred threads but not so many that this test should take more than a few seconds on a resource-starved host. Likewise, this shouldn't impose a significant memory burden. (Each interpreter uses ~3kB.)

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.

cpu - Used for certain CPU-heavy tests.

AFAIK, Conventionally, most CPU-intensive tests consume a few seconds in high-performance computers, we added the flag for low-performance CPUs to skip this test.

I thought that this test look quite CPU-intensive test, so I suggested adding it.

Comment thread Lib/test/test_interpreters.py
@corona10

Copy link
Copy Markdown
Member

Left some minor suggestions.

@ambv
ambv merged commit adda43d into python:main Jul 22, 2023
@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @ericsnowcurrently for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@bedevere-bot

Copy link
Copy Markdown

GH-107012 is a backport of this pull request to the 3.12 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 22, 2023
…ythonGH-106966)

(cherry picked from commit adda43d)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.12 only security fixes label Jul 22, 2023
@ericsnowcurrently
ericsnowcurrently deleted the interpreter-stress-tests branch July 24, 2023 15:08
ericsnowcurrently added a commit that referenced this pull request Jul 25, 2023
…H-106966) (gh-107012)

gh-105699: Add some stress tests for subinterpreter creation (GH-106966)
(cherry picked from commit adda43d)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
@bedevere-bot

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants