8000 Use max_cpu when RUBY_MAX_CPU given by riseshia · Pull Request #9399 · ruby/ruby · GitHub
[go: up one dir, main page]

Skip to content

Use max_cpu when RUBY_MAX_CPU given #9399

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

Merged
merged 1 commit into from
Jan 1, 2024
Merged

Conversation

riseshia
Copy link
Member
@riseshia riseshia commented Dec 31, 2023

Happy new year!!

From Ruby 3.3.0, M:N Thread introduced, and users can set how many CPU can be used for it via RUBY_MAX_CPU.

When user gives max cpu num for M:N thread via RUBY_MAX_CPU env, however, current implementation set value to max_cpu and overwrite it with default_max_cpu.
I think which is weird, and expected behavior is like this diff, set given max cpu num.
How do you think @ko1 ? Am I misunderstanding something? 🤔

Confirmed on custom build with this patch, and I noticed RUBY_MAX_CPU=1 make process unreachable, do we ignore if 1 given?

Comment on lines -1688 to +1692
if (USE_MN_THREADS && max_cpu_cstr && (max_cpu = atoi(max_cpu_cstr)) > 0) {
max_cpu = default_max_cpu;
if (USE_MN_THREADS && max_cpu_cstr) {
int given_max_cpu = atoi(max_cpu_cstr);
if (given_max_cpu > 0) {
max_cpu = given_max_cpu;
}
Copy link
Member

Choose a reason for hiding this comment

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

Probably (max_cpu = atoi(max_cpu_cstr)) <= 0) was intended, I guess.

Copy link
Contributor

Choose a reason for hiding this comment

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

just remove = default_max_cpu is intended. But @riseshia san's patch is more readable.

@ko1 ko1 merged commit 9368782 into ruby:master Jan 1, 2024
@ko1
Copy link
Contributor
ko1 commented Jan 1, 2024

Thank you!

@riseshia
Copy link
Member Author
riseshia commented Jan 2, 2024

Thanks!!

@riseshia riseshia deleted the use-given-max-cpu branch January 2, 2024 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0