Skip to content

Drop custom implementation of Fiber::ExecutionContext::Concurrent#16135

Merged
straight-shoota merged 3 commits intocrystal-lang:masterfrom
ysbaddaden:feature/concurrent-context-extends-parallel-context
Sep 11, 2025
Merged

Drop custom implementation of Fiber::ExecutionContext::Concurrent#16135
straight-shoota merged 3 commits intocrystal-lang:masterfrom
ysbaddaden:feature/concurrent-context-extends-parallel-context

Conversation

@ysbaddaden
Copy link
Collaborator

The Concurrent implementation mostly duplicates the Parallel one, and the parallel schedulers now take a few shortcuts when parallelism is set to 1 (skip stealing, quick evloop check) and Concurrent doesn't exhibit any noticeable performance improvement over Parallel.

Having a single implementation instead of two, almost identical, will ease maintenance.

I kept the type and didn't deprecate it yet. It's only value is that you can't mistakenly resize the context and an exception is raised when you try to (related to #15956).

@straight-shoota straight-shoota added this to the 1.18.0 milestone Sep 5, 2025
Copy link

@mverzilli mverzilli left a comment

Choose a reason for hiding this comment

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

Gotta love those removed / added line ratios 😍

@ysbaddaden
Copy link
Collaborator Author

NOTE: since the class overrides #resize we should merge after #15956.

@straight-shoota straight-shoota added this to the 1.18.0 milestone Sep 10, 2025
@straight-shoota straight-shoota merged commit 6427110 into crystal-lang:master Sep 11, 2025
39 checks passed
@github-project-automation github-project-automation bot moved this from Review to Done in Multi-threading Sep 11, 2025
@ysbaddaden ysbaddaden deleted the feature/concurrent-context-extends-parallel-context branch September 11, 2025 09:40
straight-shoota added a commit that referenced this pull request Sep 15, 2025
The concurrent context doesn't exhibit any performance improvement over the parallel context (see #16135). So we may as well use a parallel context as default context, and allow resizing it for more parallelism (see #15956).

The default configuration is without parallelism (MT:1), thus the execution context remains concurrent only, which is backward compatible (nothing changes).

Resizing the context to enable parallelism will be at the discretion of the application: it can keep using the `CRYSTAL_WORKERS` environment variable, and/or fallback to the number of CPUs or use a hardcoded value, and/or provide a command line argument, or anything else.

Co-authored-by: Johannes Müller <straightshoota@gmail.com>
ysbaddaden added a commit to ysbaddaden/crystal that referenced this pull request Oct 9, 2025
straight-shoota added a commit that referenced this pull request Oct 13, 2025
Co-authored-by: Johannes Müller <straightshoota@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants