Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Do not depend on mutex #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jeehoonkang
Copy link

This PR removes dependence on another mutex, and instead use the seq: AtomicUsize variable in Seqlock as the mutex variable.

@jeehoonkang
Copy link
Author

CI test fails because this PR uses compare_and_swap_weak, which is not stabilized in Rust 1.9 yet (https://travis-ci.org/Amanieu/seqlock/jobs/265021095#L270). Can I ask why you picked Rust 1.9 as an additional test target?

@Amanieu
Copy link
Owner

Amanieu commented Aug 16, 2017

No reason in particular. I don't mind bumping the minimum Rust version up.

However I'm not sure about replacing the mutex with what is effectively a spin lock. Do you have a reason for wanting to avoid a mutex here?

@jeehoonkang
Copy link
Author

Hmm, I just assumed that Mutex used here is a plain spinlock, but it actually is more sophisticated than that: https://github.com/Amanieu/parking_lot/blob/master/src/raw_mutex.rs#L47 https://github.com/Amanieu/parking_lot/blob/master/src/raw_mutex.rs#L131 I am sorry I didn't realize that.

I first thought that the redundancy of the mutex and seq is bad for performance, but maybe it is the case that the use of mutex reduces the contention on seq and imporves the read performance. I will benchmark on it.

By the way, just for curiosity :) can I ask why this repo is separate from your parking_lot?

@Amanieu
Copy link
Owner

Amanieu commented Aug 16, 2017

It's not really a performance reason, it's just that the seqlock only works when there is a single writer at a time. I generally prefer proper mutexes over spinlocks because it avoids issues when a thread is descheduled while holding a lock.

By the way, just for curiosity :) can I ask why this repo is separate from your parking_lot?

Again, no real reason. It just didn't feel like a proper part of parking_lot at the time since it isn't a re-implementation of a std synchronization primitive.

nico-abram added a commit to nico-abram/seqlock that referenced this pull request Oct 25, 2020
nico-abram added a commit to nico-abram/seqlock that referenced this pull request Oct 25, 2020
nico-abram added a commit to nico-abram/seqlock that referenced this pull request Oct 25, 2020
nico-abram added a commit to nico-abram/seqlock that referenced this pull request Oct 25, 2020
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.

None yet

2 participants