Skip to content

Move Thread#set_current_thread to Fiber#14099

Merged
straight-shoota merged 1 commit intocrystal-lang:masterfrom
ysbaddaden:fix/mt/move-set-current-thread-to-fiber
Dec 20, 2023
Merged

Move Thread#set_current_thread to Fiber#14099
straight-shoota merged 1 commit intocrystal-lang:masterfrom
ysbaddaden:fix/mt/move-set-current-thread-to-fiber

Conversation

@ysbaddaden
Copy link
Collaborator

This avoids manipulating fiber.@current_thread which ain't very pretty.

Crystal::Scheduler is now entirely responsible for setting the current thread. It's also responsible to verify that a making sure a fiber will always be enqueued or resumed on the thread it's been associated to, which is only affecting the #resume(fiber) and #yield(fiber) methods that are barely used.

Lastly, we remove the current_thread store that was always replacing any previous value on context swap. Sadly, this doesn't seem to have any noticeable impact on performance.

This avoids manipulating `fiber.@current_thread` which ain't very
pretty.

Crystal::Scheduler is now entirely responsible for setting the current
thread. It's also responsible to verify that a making sure a fiber will
always be enqueued or resumed on the thread it's been associated to,
which is only affecting the `#resume(fiber)` and `#yield(fiber)` methods
that are barely used.

Lastly, we remove the current_thread store that was always replacing any
previous value on context swap. Sadly, this doesn't seem to have any
noticeable impact on performance.
@straight-shoota straight-shoota added this to the 1.11.0 milestone Dec 19, 2023
@straight-shoota straight-shoota changed the title MT: move set_current_thread to Fiber Move Thread#set_current_thread to Fiber Dec 20, 2023
@straight-shoota straight-shoota merged commit f1f7848 into crystal-lang:master Dec 20, 2023
@ysbaddaden ysbaddaden deleted the fix/mt/move-set-current-thread-to-fiber branch December 21, 2023 09:30
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.

4 participants