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

Add :max_threads(*|Inf) as option to ThreadPoolScheduler #4931

Closed
wants to merge 1 commit into from

Conversation

lizmat
Copy link
Contributor

@lizmat lizmat commented May 20, 2022

For those of us brave enough to not want to be stopped by a maximum
number of OS threads. Specifying * or Inf will internally store the
value 9223372036854775807 (aka the current maximum value for an uint
attribute). The accessor will return Inf if this value is found.

For those of us brave enough to not want to be stopped by a maximum
number of OS threads.  Specifying * or Inf will internally store the
value 9223372036854775807 (aka the current maximum value for an uint
attribute).  The accessor will return Inf if this value is found.
@lizmat lizmat requested review from niner and vrurg May 20, 2022 20:58
submethod TWEAK(:$initial_threads, :$max_threads --> Nil) {
$!initial_threads = .Int with $initial_threads;
$!max_threads = nqp::istype($max_threads,Whatever)
?? 9223372036854775807 # XXX should be -1
Copy link
Member

Choose a reason for hiding this comment

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

Just a suggestion. Perhaps, make the number a constant?

?? $max_threads == Inf
?? 9223372036854775807 # XXX should be -1
!! $max_threads.Int
!! (%*ENV<RAKUDO_MAX_THREADS> // (Kernel.cpu-cores * 8 max 64)).Int;
Copy link
Member

Choose a reason for hiding this comment

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

Also just a thought. Maybe set to "infinite" if the environment is -1 or just any negative? Could be helpful in detecting some problems too.

vrurg added a commit that referenced this pull request Jun 3, 2022
Extending PR #4931.

Aside of just setting `$!max_threads` to a specific number via
`RAKUDO_MAX_THREADS` environment variable the following special cases
are now supported too:

- `RAKUDO_MAX_THREADS=` or `RAKUDO_MAX_THREADS=0` to use the default
  value
- `RAKUDO_MAX_THREADS=-1`, `RAKUDO_MAX_THREADS=inf`, or
  `RAKUDO_MAX_THREADS=unlimited` to use unlimited number of treads
- `RAKUDO_MAX_THREADS=<integer>` works as it always was
- Any other value would result in an error

Since a new scheduler is normally created at startup and exception
handling is barely needed its constructor currently throws just
`X::AdHoc`.

Note that the above special cases are now also supported for
constructor's `:max_threads` named argument:

    ThreadPoolScheduler.new(:max_threads(-1)); # Inf
    ThreadPoolScheduler.new(:max_threads<unlimited>); # Inf
    ThreadPoolScheduler.new(:max_threads(0)); # Same as omitting the
                                              # argument

Super-micro-optimization: use of BUILDPLAN is considered too much for
the `ThreadPoolScheduler` class. So, replaced it with `new`+`!SET-SELF`
approach.
@vrurg
Copy link
Member

vrurg commented Jun 3, 2022

@lizmat I would close this PR as superseded by #4941, if you don't mind.

@lizmat lizmat closed this Jun 3, 2022
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.

None yet

2 participants