-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Unified lock types for rustpython_common #2239
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
Conversation
9a7e027
to
7a39500
Compare
mod cell_lock; | ||
pub use cell_lock::{RawCellMutex as RawMutex, RawCellRwLock as RawRwLock}; | ||
|
||
pub use once_cell::unsync::{Lazy, OnceCell}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this also belong to lock?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense there; sync::OnceCell
at least has to lock when it's initializing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And it was there when this was cell.rs
as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not understanding this very well. Any other reviewer for this?
If possible, merging #2226 first maybe helpful. @qingshi163 experienced a lot of rebasing for that already
I suppose so, I'd just like to have them merged close together so that we don't run into issues with the soundness hole. |
I'm gonna merge this; I feel like |
For #2226 and specifically in order to implement
BorrowedValue::map()
; I realized that we can just make our ownRawMutex
/RawRwLock
and use it to define the Mutex types. These should be faster, too,since RefCell has more overhead (immutable accessor count mainly,since they're more direct with the use case -- e.g. just one writer vs RefCell supports multiple withCell<isize>
vsCell<bool>
)RefMut::map_split
. Turns out we can't actually make it a bool, since we subtly depend on PyRwLock having a recursive locking behavior, even though that's not guaranteed by parking_lot -- "Note that attempts to recursively acquire a read lock on aRwLock
when the current thread already holds one may result in a deadlock."