Skip to content

Change Mutex to use SRWLock on Windows. #20367

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

Merged
merged 1 commit into from
Jan 13, 2015
Merged

Conversation

retep998
Copy link
Member

Also adjusted some of the FFI definitions because apparently they don't use the long pointer prefix.
Gives a free performance boost because SRWLock is several times faster than CriticalRegion on every Windows system tested.
Fixes #19962

@rust-highfive
Copy link
Contributor

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@retep998
Copy link
Member Author

cc @alexcrichton

@alexcrichton
Copy link
Member

Can you add a comment in the code as well explaining why one might expect us to use a CRITICAL_SECTION but we're instead using a SRWLOCK?

Other than that though r=me, nice find!

@retep998 retep998 force-pushed the master branch 2 times, most recently from 91efcdc to 6b21ee0 Compare December 31, 2014 09:46
@retep998
Copy link
Member Author

Comment added and rebased.
r?

@retep998
Copy link
Member Author

We already use the unfair SRWLock for RWLock and the documentation makes no claims about fairness.
I can't find any solid information on the fairness of pthread_mutex and pthread_rwlock.

@retep998
Copy link
Member Author

I'd be fine with holding off on this PR until a design consensus is made regarding the fairness of the locks that Rust provides. In the meantime someone can add a simple flag/semaphore/whatever to Mutex to fix the recursive locking issue.

@alexcrichton
Copy link
Member

When stabilizing the std::sync::Mutex interfaces, the fairness of the primitive came up, but it was decided that for now we're not going to provide any guarantee of fairness either fair or unfair (largely pending investigation like this on various systems). For now the purpose of this type is just to serve as a mutual exclusion primitive.

The current plan is to definitely allow for these primitives to grow in the future with perhaps a new FairMutex type or a Mutex::new_fair constructor or something like that. We also have room before the final release to refine the fairness semantics of a mutex and explicitly define it as "fast and unfair" or fair, but fixing a soundness hole on Windows is likely most important for now.

@retep998 retep998 force-pushed the master branch 2 times, most recently from 54cc177 to b939b89 Compare January 2, 2015 20:38
@alexcrichton
Copy link
Member

Needs a rebase

@retep998
Copy link
Member Author

retep998 commented Jan 3, 2015

Rebased

@alexcrichton
Copy link
Member

@retep998 needs a rebase

@retep998
Copy link
Member Author

Rebased.

Signed-off-by: Peter Atashian <retep998@gmail.com>
@retep998
Copy link
Member Author

Rebased again...

bors added a commit that referenced this pull request Jan 13, 2015
Also adjusted some of the FFI definitions because apparently they don't use the long pointer prefix.
Gives a free performance boost because `SRWLock` is several times faster than `CriticalRegion` on every Windows system tested.
Fixes #19962
@bors bors merged commit ee1ca88 into rust-lang:master Jan 13, 2015
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.

Mutexes are recursive on windows, but not on unix
5 participants