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

Once implementation is available, wherever appropriate, replace futures::lock::Mutex with futures::lock::RwLock #9

Closed
jstuczyn opened this issue Dec 18, 2019 · 2 comments · Fixed by #125
Assignees

Comments

@jstuczyn
Copy link
Contributor

jstuczyn commented Dec 18, 2019

Basically wait a week or so for 0.6.0 release of https://github.com/asomers/futures-locks that will use tokio 0.2.0.
Why we want to change it to RwLock rather than Mutex? As there are way many more reads of the data compared to writes. It is read every single client request (for EVERY client) while it is written to only once per single client - during registration. So that would be a significant performance gain.

Edit: This issue refers to uses in sfw-provider as well as in nym-client

@jstuczyn jstuczyn self-assigned this Dec 18, 2019
@futurechimp futurechimp transferred this issue from another repository Jan 7, 2020
@futurechimp futurechimp added this to the 0.4.0 milestone Jan 20, 2020
@futurechimp futurechimp moved this from Cold Storage to Backlog in Core systems Jan 20, 2020
@jstuczyn
Copy link
Contributor Author

Or just replace it with tokio::sync::RwLock
Also, use tokio::sync::Mutex in place of futures::lock::Mutex for consistency sake.

@jstuczyn
Copy link
Contributor Author

Future note to whoever is going to be implementing this. tokio::sync::RwLock (from what I've tried) does not seem to have an easy access to poll method - it might be problematic if for some reason we wanted that. It might be worth to instead wait for proper futures::lock::RwLock (as with their current futures::lock::Mutex

@jstuczyn jstuczyn changed the title Change Mutex on ClientProcessing data to RwLock once library updates Once implementation is available, wherever appropriate, replace futures::lock::Mutex with futures::lock::RwLock Jan 23, 2020
@jstuczyn jstuczyn moved this from Backlog to Cold Storage in Core systems Jan 23, 2020
@jstuczyn jstuczyn removed this from the 0.4.0 milestone Jan 23, 2020
Core systems automation moved this from Cold Storage to Done Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Core systems
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants