Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove the simple-mutex dependency #10

Merged
merged 2 commits into from
Sep 21, 2023
Merged

Remove the simple-mutex dependency #10

merged 2 commits into from
Sep 21, 2023

Conversation

notgull
Copy link
Member

@notgull notgull commented Sep 21, 2023

It appears that this was originally done when std::sync::Mutex used the PThreads implementation, which was much slower than other contemporary Mutex implementations (like parking_lot). Therefore it made sense to use a custom Mutex implementation here. However std Mutexes now use futexes, which are much faster than the previous implementation.

In addition, the original git history for simple-mutex appears to be lost to time. The only copy of the source code is on crates.io, and the crate is not owned by anyone. So it is problematic to use it anyways, as updating it would require us to go through the painstaking process of reclaiming the name.

This commit removes the dependency on simple-mutex and replaces its usages with std::sync::Mutex for the reasons listed above. The performance impact of this change has not been measured, but I believe it to be negligible.

It appears that this was originally done when std::sync::Mutex used the
PThreads implementation, which was much slower than other
contemporary Mutex implementations (like parking_lot). Therefore it
made sense to use a custom Mutex implementation here. However std
Mutexes now use futexes, which are much faster than the previous
implementation.

In addition, the original git history for simple-mutex appears to be lost
to time. The only copy of the source code is on crates.io, and the crate
is not owned by anyone. So it is problematic to use it anyways, as
updating it would require us to go through the painstaking process of
reclaiming the name.

This commit removes the dependency on simple-mutex and replaces its
usages with std::sync::Mutex for the reasons listed above. The
performance impact of this change has not been measured, but I believe
it to be negligible.

Signed-off-by: John Nunley <dev@notgull.net>
Signed-off-by: John Nunley <dev@notgull.net>
Copy link
Collaborator

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

LGTM

@notgull notgull merged commit 50851bf into master Sep 21, 2023
@notgull notgull deleted the notgull/nodep branch September 21, 2023 02:29
@notgull notgull mentioned this pull request Sep 25, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants