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

corona10 · 2023-09-21T16:45:41Z

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

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

corona10 · 2023-09-21T16:46:39Z

corona10 · 2023-09-21T16:59:19Z

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 :)

Doc/library/os.rst

Doc/using/cmdline.rst

Include/cpython/initconfig.h

Doc/using/cmdline.rst

Python/initconfig.c

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

vstinner · 2023-09-21T17:51:20Z

Programs/_testembed.c

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

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


Can you please set a more interesting value like 1234?

vstinner

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

vstinner · 2023-09-21T18:01:42Z

Doc/using/cmdline.rst

+   * :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.


Suggested change

* :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.

* :samp:`-X cpu_count={n}` overrides the number of CPU count from system:

:func:`os.cpu_count` returns *n*. The value *n* must be greater than

or equal to 1.

Doc/using/cmdline.rst

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

Python/initconfig.c

vstinner · 2023-09-21T18:05:52Z

Python/initconfig.c

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


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

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

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

If you want, you can 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

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

Modules/posixmodule.c

vstinner · 2023-10-02T16:13:40Z

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: see https://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 is scheduled for Tuesday, 2023-10-17, we will have time. Also, it may wait for alpha 2 ;-)

Lib/os.py

Modules/posixmodule.c

Python/sysmodule.c

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

vstinner · 2023-10-09T20:35:54Z

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

corona10 · 2023-10-09T23:25:37Z

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

vstinner

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

vstinner · 2023-10-10T10:25:56Z

Congrats.

gpshead · 2023-10-10T11:13:19Z

🎉 thanks everybody!

corona10 · 2023-10-10T21:20:22Z

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

vstinner · 2023-10-10T21:36:11Z

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

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

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

41be170

corona10 requested a review from vstinner September 21, 2023 16:45

bedevere-app bot added the awaiting review label Sep 21, 2023

bedevere-app bot mentioned this pull request Sep 21, 2023

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

Closed

corona10 added 2 commits September 22, 2023 01:47

Check style

f7a7428

Add help

7009bbe

vstinner reviewed Sep 21, 2023

View reviewed changes

corona10 and others added 8 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 from vstinner September 21, 2023 17:48

Update NEWS.d

95a2173

vstinner reviewed Sep 21, 2023

View reviewed changes

corona10 added 6 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

corona10 and others added 3 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 added 2 commits October 2, 2023 10:24

Use overridden

75021be

Minor refactoring

57dd53b

corona10 commented Oct 2, 2023

View reviewed changes

Modules/posixmodule.c Outdated Show resolved Hide resolved

corona10 added 2 commits October 2, 2023 20:02

Revert to PYTHONCPUCOUNT

3f9da50

fix

57e82c5

Use PYTHON_CPU_COUNT

c37c8d0

gpshead requested changes Oct 3, 2023

View reviewed changes

Lib/os.py Outdated Show resolved Hide resolved

Modules/posixmodule.c Outdated Show resolved Hide resolved

Address Greg's review

c7726e5

corona10 requested a review from gpshead October 3, 2023 02:12

corona10 commented Oct 3, 2023

View reviewed changes

Python/sysmodule.c Show resolved Hide resolved

corona10 added 2 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.13 bugs and security fixes labels Oct 3, 2023

gpshead and others added 3 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' into pythongh-109595

a0cfb21

Merge remote-tracking branch 'upstream/main' into pythongh-109595

8daa3b7

vstinner approved these changes Oct 10, 2023

View reviewed changes

corona10 merged commit 0362cbf into python:main Oct 10, 2023

corona10 deleted the gh-109595 branch October 10, 2023 10:00

bedevere-app bot removed the awaiting core review label Oct 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

corona10 commented Sep 21, 2023 •

edited by github-actions bot

Loading

corona10 commented Sep 21, 2023

corona10 commented Sep 21, 2023 •

edited

Loading

vstinner Sep 21, 2023

vstinner left a comment

vstinner Sep 21, 2023

vstinner Sep 21, 2023

corona10 Sep 21, 2023

vstinner Sep 21, 2023

vstinner commented Oct 2, 2023

vstinner commented Oct 9, 2023

corona10 commented Oct 9, 2023 •

edited

Loading

vstinner left a comment

vstinner commented Oct 10, 2023

gpshead commented Oct 10, 2023

corona10 commented Oct 10, 2023

vstinner commented Oct 10, 2023

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

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

Conversation

corona10 commented Sep 21, 2023 • edited by github-actions bot Loading

corona10 commented Sep 21, 2023

corona10 commented Sep 21, 2023 • edited Loading

vstinner Sep 21, 2023

Choose a reason for hiding this comment

vstinner left a comment

Choose a reason for hiding this comment

vstinner Sep 21, 2023

Choose a reason for hiding this comment

vstinner Sep 21, 2023

Choose a reason for hiding this comment

corona10 Sep 21, 2023

Choose a reason for hiding this comment

vstinner Sep 21, 2023

Choose a reason for hiding this comment

vstinner commented Oct 2, 2023

vstinner commented Oct 9, 2023

corona10 commented Oct 9, 2023 • edited Loading

vstinner left a comment

Choose a reason for hiding this comment

vstinner commented Oct 10, 2023

gpshead commented Oct 10, 2023

corona10 commented Oct 10, 2023

vstinner commented Oct 10, 2023

corona10 commented Sep 21, 2023 •

edited by github-actions bot

Loading

corona10 commented Sep 21, 2023 •

edited

Loading

corona10 commented Oct 9, 2023 •

edited

Loading