Replace custom spinlock with spin #41
Conversation
|
Hmm, why do we have spinlock? Sorry for not being able to review your recent PRs, but I remember I disagreed with spinlock in a no-std related issue in this repository -- AFAIK, using spinlock around pure ops like arithmetic operations is fine, but using it around allocations is not always safe (may cause a deadlock). In fact, there is a report that the spinlock around the allocations at crossbeam-channel (recently removed) is believed to have caused a deadlock. |
|
Ah, it seems the current implementation doesn't have a problem with deadlock as it is try_lock only and the spinning is not unbounded. |
|
Yes, the main idea is that it uses a spin lock in the case where it's theoretically ideal and falls back to an atomic queue during periods of high contention. I wrote a more detailed write-up in #34. (Also, don't apologize for not being able to review my PRs. I know you're busy, and your well-being is more important than my code. 🙂) |
|
Note than |
| @@ -25,6 +25,7 @@ __test = [] | |||
| crossbeam-utils = { version = "0.8.12", default-features = false } | |||
| parking = { version = "2.0.0", optional = true } | |||
| slab = { version = "0.4.7", default-features = false } | |||
| spin = { version = "0.9.4", default-features = false, features = ["spin_mutex"] } | |||
There was a problem hiding this comment.
imo it would be more readable to replace this with
[dependencies.spin]
version = "0.9.4"
default-features = false
features = ["spin_mutex"]| fn drop(&mut self) { | ||
| self.mutex.locked.store(false, Ordering::Release); | ||
| } | ||
| guard: spin::MutexGuard<'a, T>, |
There was a problem hiding this comment.
wouldn't it make more sense to just do use spin::MutexGuard; here`
|
Taking another look at this, I'm not a fan of incurring the extra dependency. |
This deduplicates the custom spinlock we have with the one that's currently used in the
spincrate.