Skip to content

Conversation

ysbaddaden
Copy link
Contributor

@ysbaddaden ysbaddaden commented Sep 9, 2025

  • Makes sure there is at least 1 worker (minimum required).
  • Doesn't cap the total to an arbitrary number.
  • Caps the total to the effective logical CPUs when supported.

Depends on #16148.

@yxhuvud
Copy link
Contributor

yxhuvud commented Sep 9, 2025

Caps the total to the effective logical CPUs when supported (linux).

This I feel would be a mistake. While from a pure processing point of view it would probably be ok, there are other situations where you do outgoing calls in some way that may block execution but might not be very costly from a CPU perspective. The classical example would be a thread pool having a bunch of threads reading from spinning disks in parallel - then it may make sense to overcommit by a lot. I suppose the usefulness of that particular example goes down when using io_uring as the event loop, but there are other situations where it may make sense.

EDIT: Oh, this caps only the DEFAULT value? Ok, that make a lot of sense. Never mind then

@ysbaddaden
Copy link
Contributor Author

It's not even the actual default parallelism, it's the one we recommend when you opt in to MT by resizing the default context.

- Makes sure there is at least 1 worker (minimum required).
- Doesn't cap the total to an arbitrary number.
- Caps the total to the effective logical CPUs when supported (linux).
@ysbaddaden ysbaddaden force-pushed the fix/ec-dont-cap-default-workers-count branch from 70ddc98 to f619d2f Compare September 11, 2025 11:36
@straight-shoota straight-shoota added this to the 1.18.0 milestone Sep 19, 2025
@straight-shoota straight-shoota merged commit 0be6ee6 into crystal-lang:master Sep 21, 2025
39 checks passed
@github-project-automation github-project-automation bot moved this from Review to Done in Multi-threading Sep 21, 2025
@ysbaddaden ysbaddaden deleted the fix/ec-dont-cap-default-workers-count branch September 21, 2025 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants