NotificationsYou must be signed in to change notification settings
Fork32.1k
Star67.3k

gh-109595: Add -Xcpu_count=<n> cmdline for container users#109667

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

corona10 merged 71 commits intopython:mainfromcorona10:gh-109595

Oct 10, 2023

Merged

gh-109595: Add -Xcpu_count=<n> cmdline for container users#109667

corona10 merged 71 commits intopython:mainfromcorona10:gh-109595

Oct 10, 2023

Conversation

Copy link

Member

corona10 commentedSep 21, 2023•
edited by github-actionsbot
Loading

Issue:Support python -Xcpu_count=<n> feature for container environment. #109595

📚 Documentation preview 📚:https://cpython-previews--109667.org.readthedocs.build/

pythongh-109595: Add -Xcpu_count=<n> cmdline for container users

41be170

corona10 requested a review fromvstinner

September 21, 2023 16:45

bedevere-appbot added the awaiting review label

Sep 21, 2023

bedevere-appbot mentioned this pull request

Sep 21, 2023

Support python -Xcpu_count=<n> feature for container environment.#109595

Closed

Copy link

MemberAuthor

corona10 commentedSep 21, 2023

cc@indygreg

corona10 added2 commits

September 22, 2023 01:47

Check style

f7a7428

Add help

7009bbe

Copy link

MemberAuthor

corona10 commentedSep 21, 2023•
edited
Loading

Please read the issue of why this flag is needed and see also actual use-case from#109595 (comment)

Even if the container system is important these days, there is no way to limit CPU resources for the container environment from Python program side.
JDK provides-XX:ActiveProcessorCount=<n> from JDK 21 instead of supporting cgroup, this patch will provide identical feature to Python user and super useful :)

vstinner reviewed

Sep 21, 2023

View reviewed changes

Doc/library/os.rst OutdatedShow resolvedHide resolved

Doc/using/cmdline.rst OutdatedShow resolvedHide resolved

Include/cpython/initconfig.hShow resolvedHide resolved

Doc/using/cmdline.rst OutdatedShow resolvedHide resolved

Python/initconfig.c OutdatedShow resolvedHide resolved

corona10and others added8 commits

September 22, 2023 02:23

Apply suggestions from code review

d1f91d8

Co-authored-by: Victor Stinner <vstinner@python.org>

Apply suggestions from code review

8c92ed6

Co-authored-by: Victor Stinner <vstinner@python.org>

Check style

c27bdfc

Address code review

b13e5ee

Address code review

45fce16

Address code review

49a48e4

Address code review

829a8e8

Fix test

cfb33a4

corona10 requested a review fromvstinner

September 21, 2023 17:48

Update NEWS.d

95a2173

vstinner reviewed

Sep 21, 2023

View reviewed changes

Programs/_testembed.c Outdated

		@@ -715,6 +715,7 @@ static int test_init_from_config(void)

		putenv("PYTHONINTMAXSTRDIGITS=6666");
		config.int_max_str_digits = 31337;
		config.cpu_count = -1;

Copy link

Member

vstinnerSep 21, 2023

Choose a reason for hiding this comment

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

Can you please set a more interesting value like 1234?

corona10 added6 commits

September 22, 2023 02:56

Address code review

0394d1d

Update

f4a3f01

Update

60a28fe

nit

7e5595c

Update

8678012

Add PYTHONCPUCOUNT

cbc5484

vstinner reviewed

Sep 21, 2023

View reviewed changes

Copy link

Member

vstinner left a comment

Choose a reason for hiding this comment

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

Can you please document the change in Doc/whatsnew/3.13.rst?

Doc/using/cmdline.rst Outdated

Comment on lines 549 to 551

		* :samp:`-X cpu_count={n}` will override the number of CPU count from system.
		If this option is provided, :func:`os.cpu_count` will return the overrided value.
		And n must be greater equal then 1.

Copy link

Member

vstinnerSep 21, 2023

Choose a reason for hiding this comment

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

Suggested change

	*:samp:`-X cpu_count={n}`will overridethe number of CPU count from system.
	If this option is provided,:func:`os.cpu_count`will return the overridedvalue.
	And n must be greaterequalthen 1.
	*:samp:`-X cpu_count={n}`overridesthe number of CPU count from system:
	:func:`os.cpu_count`returns n. Thevalue n must be greater than
	orequalto 1.

Doc/using/cmdline.rst OutdatedShow resolvedHide resolved

Misc/NEWS.d/next/Core and Builtins/2023-09-22-01-44-53.gh-issue-109595.fVINgD.rst OutdatedShow resolvedHide resolved

Python/initconfig.cShow resolvedHide resolved

Python/initconfig.c Outdated

		static PyStatus
		config_init_cpu_count(PyConfig *config)
		{
		int cpu_count = -1;

Copy link

Member

vstinnerSep 21, 2023

Choose a reason for hiding this comment

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

You can move the variable declaration inside the "if (sep)" block. Does ithave to be initialized?

If you are afraid of undefined behavior, maybe config_wstr_to_int() should set*result to 0 on error.

Copy link

MemberAuthor

corona10Sep 21, 2023

Choose a reason for hiding this comment

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

Now I addedPYTHONCPUCOUNT envvar, so the current structure will be needed.

Copy link

Member

vstinnerSep 21, 2023

Choose a reason for hiding this comment

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

If you want, youcan move the variable declaration inside each "if (env)" block and duplicate the variable, to better show its scope. But well, that's just a personal preference. Feel free to ignore my coding style remark ;-)

Python/initconfig.c OutdatedShow resolvedHide resolved

corona10and others added3 commits

September 22, 2023 03:13

Add PYTHONCPUCOUNT

4622b60

Apply suggestions from code review

50f0178

Co-authored-by: Victor Stinner <vstinner@python.org>

Update Python/initconfig.c

5c3ac68

Co-authored-by: Victor Stinner <vstinner@python.org>

corona10 added2 commits

October 2, 2023 10:24

Use overridden

75021be

Minor refactoring

57dd53b

corona10 commented

Oct 2, 2023

View reviewed changes

Modules/posixmodule.c OutdatedShow resolvedHide resolved

corona10 added2 commits

October 2, 2023 20:02

Revert to PYTHONCPUCOUNT

3f9da50

fix

57e82c5

Copy link

Member

vstinner commentedOct 2, 2023

I'm fine with os._get_cpu_count_config() returning a string.

For the env var, apparently, using an underscore or not became a hot debate: seehttps://discuss.python.org/t/change-environment-variable-style/35180 discussion. Maybe we should wait a few days until the dust settle down to see if a consensus is reached? Python 3.13.0 alpha 1 isscheduled for Tuesday, 2023-10-17, we will have time. Also, it may wait for alpha 2 ;-)

Use PYTHON_CPU_COUNT

c37c8d0

gpshead requested changes

Oct 3, 2023

View reviewed changes

Lib/os.py OutdatedShow resolvedHide resolved

Modules/posixmodule.c OutdatedShow resolvedHide resolved

Address Greg's review

c7726e5

corona10 requested a review fromgpshead

October 3, 2023 02:12

corona10 commented

Oct 3, 2023

View reviewed changes

Python/sysmodule.cShow resolvedHide resolved

corona10 added2 commits

October 3, 2023 11:23

Address Greg's code review

de8bf53

nit

633914b

gpshead added type-feature

A feature request or enhancement

docs

Documentation in the Doc dir

3.13bugs and security fixes labels

Oct 3, 2023

gpsheadand others added3 commits

October 3, 2023 04:57

Include multiprocessing in docs, reword.

37fbdfe

Adds PYTHON_CPU_COUNT help text.Mentions multiprocessing.cpu_count.rewords a few statements.

Merge remote-tracking branch 'upstream/main' intopythongh-109595

a0cfb21

Merge remote-tracking branch 'upstream/main' intopythongh-109595

8daa3b7

Copy link

Member

vstinner commentedOct 9, 2023

The majority wants PYTHON_CPU_COUNT env name:https://discuss.python.org/t/change-environment-variable-style/35180

Copy link

MemberAuthor

corona10 commentedOct 9, 2023•
edited
Loading

@gpshead I will merge this PR by tomorrow. Please let me know if there needs more change. :)
cc@vstinner

vstinner approved these changes

Oct 10, 2023

View reviewed changes

Copy link

Member

vstinner left a comment

Choose a reason for hiding this comment

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

LGTM. I see that you addressed the two@gpshead's comments. You can merge.

corona10 merged commit0362cbf intopython:main

Oct 10, 2023

corona10 deleted the gh-109595 branch

October 10, 2023 10:00

bedevere-appbot removed the awaiting core review label

Oct 10, 2023

Copy link

Member

vstinner commentedOct 10, 2023

Congrats.

Copy link

Member

gpshead commentedOct 10, 2023

🎉 thanks everybody!

Copy link

MemberAuthor

corona10 commentedOct 10, 2023

Thanks@vstinner and@gpshead for all your efforts to consider every use case!!

Copy link

Member

vstinner commentedOct 10, 2023

Follow-up: issue#110649 to add -X cpu_count=process.

Glyphack pushed a commit to Glyphack/cpython that referenced this pull request

Sep 2, 2024

pythongh-109595: Add -Xcpu_count=<n> cmdline for container users (pyt…

cb98434

…hon#109667)---------Co-authored-by: Victor Stinner <vstinner@python.org>Co-authored-by: Gregory P. Smith [Google LLC] <greg@krypto.org>

Labels

3.13

bugs and security fixes

docs

Documentation in the Doc dir

type-feature

A feature request or enhancement

5 participants

Movatterモバイル変換

Uh oh!

gh-109595: Add -Xcpu_count=<n> cmdline for container users#109667

gh-109595: Add -Xcpu_count=<n> cmdline for container users#109667

Uh oh!

Conversation

corona10 commentedSep 21, 2023• edited by github-actionsbotLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

corona10 commentedSep 21, 2023

Uh oh!

corona10 commentedSep 21, 2023• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinnerSep 21, 2023

Choose a reason for hiding this comment

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinnerSep 21, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinnerSep 21, 2023

Choose a reason for hiding this comment

Uh oh!

corona10Sep 21, 2023

Choose a reason for hiding this comment

Uh oh!

vstinnerSep 21, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vstinner commentedOct 2, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner commentedOct 9, 2023

Uh oh!

corona10 commentedOct 9, 2023• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner commentedOct 10, 2023

Uh oh!

gpshead commentedOct 10, 2023

Uh oh!

corona10 commentedOct 10, 2023

Uh oh!

vstinner commentedOct 10, 2023

Uh oh!

Uh oh!

corona10 commentedSep 21, 2023•
edited by github-actionsbot
Loading

corona10 commentedSep 21, 2023•
edited
Loading

corona10 commentedOct 9, 2023•
edited
Loading