Skip to content

Switch from crossbeam-channel to std::sync::mpsc::channel.#1924

Merged
gz merged 2 commits into
mainfrom
std-channels
Jun 24, 2024
Merged

Switch from crossbeam-channel to std::sync::mpsc::channel.#1924
gz merged 2 commits into
mainfrom
std-channels

Conversation

@gz

@gz gz commented Jun 23, 2024

Copy link
Copy Markdown
Contributor

The channel code in std is identical to crossbeam (taken from on crossbeam-channel) except that rust-lang/rust#107466 is fixed in std but not in crossbeam-channel.

I originally reworked this because the bug in 80ff3ff made me desperate to suspect something is wrong with the channel. While it didn't turn out to be an issue with the channels it's easier to depend on std code instead of a library.

Is this a user-visible change (yes/no): ___

@gz gz requested a review from blp June 23, 2024 14:07
@gz gz force-pushed the std-channels branch 2 times, most recently from 3a00841 to 41a9f19 Compare June 23, 2024 14:24
gz added 2 commits June 23, 2024 16:47
The code is identical except that
rust-lang/rust#107466
is fixed in std but not in crossbeam-channel.

I originally reworked this because the bug in 80ff3ff
made me desparate to suspect something is wrong with the
channel. While it didn't turn out to be an issue with the
channels  I still think it's good to depend on std code
instead of a library.

Signed-off-by: Gerd Zellweger <mail@gerdzellweger.com>
Signed-off-by: Gerd Zellweger <mail@gerdzellweger.com>

@blp blp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe crossbeam is faster but I doubt it has any impact on this use case.

@ryzhyk

ryzhyk commented Jun 23, 2024

Copy link
Copy Markdown
Contributor

I didn't realize they switched std to use the crossbeam channel. Do they use the crossbeam mutex as well in std or is it still a different impl?

@blp

blp commented Jun 23, 2024

Copy link
Copy Markdown
Member

Maybe crossbeam is faster but I doubt it has any impact on this use case.

Oh gosh, you say they're identical except for a bug fix. I need to read better.

@gz gz merged commit b5f9187 into main Jun 24, 2024
@gz gz deleted the std-channels branch June 24, 2024 00:50
@gz

gz commented Jun 24, 2024

Copy link
Copy Markdown
Contributor Author

I didn't realize they switched std to use the crossbeam channel. Do they use the crossbeam mutex as well in std or is it still a different impl?

Not sure, but they switched to use futex at some point
rust-lang/rust#95035

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