Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Use RwLock for state DB #7425

Merged
merged 2 commits into from
Jan 2, 2018
Merged

Use RwLock for state DB #7425

merged 2 commits into from
Jan 2, 2018

Conversation

tomusdrw
Copy link
Collaborator

@tomusdrw tomusdrw commented Jan 2, 2018

To allow parallel reads.

Had to introduce Mutex in state_db_cache though (since Account has Cell/RefCell inside which makes it !Sync). However the Mutex is guaranteed to be accessed from single thread (withing sync_cache method) and actually Account is never modified (not even read from) in that context, it should be safe to just wrap it with something that provides unsafe Sync, but IMHO better be safe with this.

@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Jan 2, 2018
@tomusdrw tomusdrw requested review from debris and arkpar January 2, 2018 09:00
@debris debris added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 2, 2018
@debris debris merged commit 7deeb26 into master Jan 2, 2018
@debris debris deleted the td-state-lock branch January 2, 2018 10:57
@5chdn 5chdn added this to the 1.9 milestone Jan 2, 2018
@@ -58,7 +58,7 @@ struct CacheQueueItem {
/// Account address.
address: Address,
/// Acccount data or `None` if account does not exist.
account: Option<Account>,
account: Mutex<Option<Account>>,
Copy link
Collaborator

@arkpar arkpar Jan 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure that creating thousands of mutexes is a good idea.

@arkpar
Copy link
Collaborator

arkpar commented Jan 3, 2018

Parallel reads were actually possible before this change. The mutex was never held for the actual read but only to make a clone.

@tomusdrw
Copy link
Collaborator Author

tomusdrw commented Jan 4, 2018

@arkpar Let me know if this is any better: #7456
if you are not happy with it we can revert the PR.

The mutex was never held for the actual read but only to make a clone.
Indeed that was the case although IMHO it's still worth to relax the requirements by using RwLock.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants