GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-109595: Add -Xcpu_count=<n> cmdline for container users by corona10 · Pull Request #109667 · python/cpython Conversation
|
|
cc @indygreg |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
|
|
||
| putenv("PYTHONINTMAXSTRDIGITS=6666"); | ||
| config.int_max_str_digits = 31337; | ||
| config.cpu_count = -1; |
There was a problem 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?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| * :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. |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
| * :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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| static PyStatus | ||
| config_init_cpu_count(PyConfig *config) | ||
| { | ||
| int cpu_count = -1; |
There was a problem 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 it have to be initialized?
If you are afraid of undefined behavior, maybe config_wstr_to_int() should set *result to 0 on error.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Now I added PYTHONCPUCOUNT envvar, so the current structure will be needed.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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 ;-)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
There was an error while loading. Please reload this page.
Adds PYTHON_CPU_COUNT help text. Mentions multiprocessing.cpu_count. rewords a few statements.
|
The majority wants PYTHON_CPU_COUNT env name: https://discuss.python.org/t/change-environment-variable-style/35180 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@gpshead I will merge this PR by tomorrow. Please let me know if there needs more change. :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Congrats. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
🎉 thanks everybody! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Follow-up: issue #110649 to add -X cpu_count=process. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…hon#109667) --------- Co-authored-by: Victor Stinner <[email protected]> Co-authored-by: Gregory P. Smith [Google LLC] <[email protected]>
|
Hello, Just a ping to say that the function doesn't work and the doc is wrong.
expected: It seems the value is only read once at interpreter startup. FYI: This is a problem because python cannot detect the number cores correctly. It's not aware of the number of cpu exposed to containers/VM and try to use all CPU from the host machine, it's not aware of the number of CPU it should use in compute clusters that expect 1 core per job. I was looking forward to setting |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
You're correct: the If you really have to change But it's better to set the environment variable before Python startup. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
I agree but it's not possible in practice.
So the fix for me is to fix the CPU detection in the For reference, this is a pretty critical issue with the last generation of server CPUs released in 2024 which have way more cores (up to 192c/384t per socket). It only take a few misbehaved apps/libraries each trying to create N subthreads/subprocesses to accidentally kill the server and other jobs. 64x64 on an older server is merely 4096 threads/processes. 512x512 on a recent server is 262144 threads/processes. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This closed issue is not the right place to ask questions on how to use Python. I suggest you finding another place to ask your question such as the Python Help category of discuss.python.org. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This thread is the right place to discuss cpu detection in the interpreter, as it was modified by this ticket and needs further improvements.
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
You can propose a PR, I don't know if it would be accepted. @corona10 added this feature.
I don't think that it's a good idea. It's common in Python to only read an environment variable once at startup.
So far, the consensus is to not read cgroup limits since there is not reliable way to get a number of CPUs. Again, this closed issue is not the right place to discuss it. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
You can propose a PR for doc. But rest of things does not look like good idea. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
morotti please open a thread on discuss.python.org ideally which could ultimately spawn new issues. closed PRs are not a meaningful place for discussions. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.