Skip to content
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

Rename num_domains to num_additional_domains in setup_pool #31

Merged
merged 3 commits into from
Jun 8, 2021

Conversation

Sudha247
Copy link
Contributor

Mentions that we actually create a pool of num_domains + 1 along with the parent domain.

@kayceesrk
Copy link
Contributor

Would it be useful to change the num_domains parameter as num_additional_domains to make it explicit? This is a breaking change. @ctk21 what are your thoughts?

@ctk21
Copy link
Contributor

ctk21 commented Jun 3, 2021

I can see a clarity advantage to cleaning that up to num_additional_domains (or num_workers) because I can see it being confusing as it is.

We will probably need to make a release of domainslib soon that switches us from using critical_section to Mutex/Condition, so getting the change in for that release might make sense.

@Sudha247 Sudha247 changed the title Minor doc update in setup_pool Rename num_domains to num_additional_domains in setup_pool Jun 4, 2021
@ctk21
Copy link
Contributor

ctk21 commented Jun 8, 2021

Looks good to me.

@ctk21 ctk21 merged commit 0a3b4f3 into ocaml-multicore:master Jun 8, 2021
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