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

Use im::HashMap for storage_changes #9465

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions ethcore/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ itertools = "0.5"
lazy_static = "1.0"
log = "0.4"
lru-cache = "0.1"
im = "12.0"
num = { version = "0.1", default-features = false, features = ["bigint"] }
num_cpus = "1.2"
parity-machine = { path = "../machine" }
Expand Down
1 change: 1 addition & 0 deletions ethcore/private-tx/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ patricia-trie-ethereum = { path = "../../util/patricia-trie-ethereum" }
rand = "0.3"
rlp = { version = "0.2.4", features = ["ethereum"] }
rlp_derive = { path = "../../util/rlp_derive" }
im = "12.0"
rustc-hex = "1.0"
serde = "1.0"
serde_derive = "1.0"
Expand Down
7 changes: 4 additions & 3 deletions ethcore/private-tx/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ extern crate transaction_pool as txpool;
extern crate patricia_trie_ethereum as ethtrie;
extern crate rlp;
extern crate url;
extern crate im;
extern crate rustc_hex;
#[macro_use]
extern crate log;
Expand All @@ -68,7 +69,7 @@ pub use messages::{PrivateTransaction, SignedPrivateTransaction};
pub use error::{Error, ErrorKind};

use std::sync::{Arc, Weak};
use std::collections::{HashMap, HashSet};
use std::collections::HashSet;
use std::time::Duration;
use ethereum_types::{H128, H256, U256, Address};
use hash::keccak;
Expand Down Expand Up @@ -452,7 +453,7 @@ impl Provider where {
.map_err(|e| ErrorKind::Call(format!("Contract call failed {:?}", e)))?)
}

fn snapshot_to_storage(raw: Bytes) -> HashMap<H256, H256> {
fn snapshot_to_storage(raw: Bytes) -> im::HashMap<H256, H256> {
let items = raw.len() / 64;
(0..items).map(|i| {
let offset = i * 64;
Expand All @@ -462,7 +463,7 @@ impl Provider where {
}).collect()
}

fn snapshot_from_storage(storage: &HashMap<H256, H256>) -> Bytes {
fn snapshot_from_storage(storage: &im::HashMap<H256, H256>) -> Bytes {
let mut raw = Vec::with_capacity(storage.len() * 64);
for (key, value) in storage {
raw.extend_from_slice(key);
Expand Down
1 change: 1 addition & 0 deletions ethcore/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ extern crate patricia_trie_ethereum as ethtrie;
extern crate triehash_ethereum as triehash;
extern crate ansi_term;
extern crate unexpected;
extern crate im;
extern crate parity_snappy as snappy;
extern crate ethabi;
extern crate rustc_hex;
Expand Down
34 changes: 20 additions & 14 deletions ethcore/src/state/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

use std::fmt;
use std::sync::Arc;
use std::collections::{HashMap, BTreeMap};
use std::collections::BTreeMap;
use hash::{KECCAK_EMPTY, KECCAK_NULL_RLP, keccak};
use ethereum_types::{H256, U256, Address};
use error::Error;
Expand All @@ -30,6 +30,7 @@ use trie::{Trie, Recorder};
use ethtrie::{TrieFactory, TrieDB, SecTrieDB, Result as TrieResult};
use pod_account::*;
use rlp::{RlpStream, encode};
use im;
use lru_cache::LruCache;
use basic_account::BasicAccount;

Expand Down Expand Up @@ -61,7 +62,7 @@ pub struct Account {
storage_cache: RefCell<LruCache<H256, H256>>,
// Modified storage. Accumulates changes to storage made in `set_storage`
// Takes precedence over `storage_cache`.
storage_changes: HashMap<H256, H256>,
storage_changes: im::HashMap<H256, H256>,
// Code hash of the account.
code_hash: H256,
// Size of the account code.
Expand All @@ -81,7 +82,7 @@ impl From<BasicAccount> for Account {
nonce: basic.nonce,
storage_root: basic.storage_root,
storage_cache: Self::empty_storage_cache(),
storage_changes: HashMap::new(),
storage_changes: im::HashMap::new(),
code_hash: basic.code_hash,
code_size: None,
code_cache: Arc::new(vec![]),
Expand All @@ -94,7 +95,7 @@ impl From<BasicAccount> for Account {
impl Account {
#[cfg(test)]
/// General constructor.
pub fn new(balance: U256, nonce: U256, storage: HashMap<H256, H256>, code: Bytes) -> Account {
pub fn new(balance: U256, nonce: U256, storage: im::HashMap<H256, H256>, code: Bytes) -> Account {
Account {
balance: balance,
nonce: nonce,
Expand Down Expand Up @@ -136,7 +137,7 @@ impl Account {
nonce: nonce,
storage_root: KECCAK_NULL_RLP,
storage_cache: Self::empty_storage_cache(),
storage_changes: HashMap::new(),
storage_changes: im::HashMap::new(),
code_hash: KECCAK_EMPTY,
code_cache: Arc::new(vec![]),
code_size: Some(0),
Expand All @@ -160,7 +161,7 @@ impl Account {
nonce: nonce,
storage_root: KECCAK_NULL_RLP,
storage_cache: Self::empty_storage_cache(),
storage_changes: HashMap::new(),
storage_changes: im::HashMap::new(),
code_hash: KECCAK_EMPTY,
code_cache: Arc::new(vec![]),
code_size: None,
Expand All @@ -184,7 +185,7 @@ impl Account {
}

/// Reset this account's code and storage to given values.
pub fn reset_code_and_storage(&mut self, code: Arc<Bytes>, storage: HashMap<H256, H256>) {
pub fn reset_code_and_storage(&mut self, code: Arc<Bytes>, storage: im::HashMap<H256, H256>) {
self.code_hash = keccak(&*code);
self.code_cache = code;
self.code_size = Some(self.code_cache.len());
Expand Down Expand Up @@ -360,7 +361,7 @@ impl Account {
pub fn storage_root(&self) -> Option<&H256> { if self.storage_is_clean() {Some(&self.storage_root)} else {None} }

/// Return the storage overlay.
pub fn storage_changes(&self) -> &HashMap<H256, H256> { &self.storage_changes }
pub fn storage_changes(&self) -> &im::HashMap<H256, H256> { &self.storage_changes }

/// Increment the nonce of the account by one.
pub fn inc_nonce(&mut self) {
Expand All @@ -382,7 +383,12 @@ impl Account {
/// Commit the `storage_changes` to the backing DB and update `storage_root`.
pub fn commit_storage(&mut self, trie_factory: &TrieFactory, db: &mut HashDB<KeccakHasher>) -> TrieResult<()> {
let mut t = trie_factory.from_existing(db, &mut self.storage_root)?;
for (k, v) in self.storage_changes.drain() {
let old_storage_changes = {
let mut tmp = im::HashMap::new();
::std::mem::swap(&mut tmp, &mut self.storage_changes);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not quite related to this specific PR, but I just wonder, what will happen if it'll fail to commit? Should we concern about this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Then that error will be propagate out and cause the block import to fail. I think we handled that correctly in Client.

tmp
};
for (k, v) in old_storage_changes {
// cast key and value to trait type,
// so we can call overloaded `to_bytes` method
match v.is_zero() {
Expand Down Expand Up @@ -429,7 +435,7 @@ impl Account {
nonce: self.nonce.clone(),
storage_root: self.storage_root.clone(),
storage_cache: Self::empty_storage_cache(),
storage_changes: HashMap::new(),
storage_changes: im::HashMap::new(),
code_hash: self.code_hash.clone(),
code_size: self.code_size.clone(),
code_cache: self.code_cache.clone(),
Expand Down Expand Up @@ -498,7 +504,7 @@ impl fmt::Debug for Account {
.field("balance", &self.balance)
.field("nonce", &self.nonce)
.field("code", &self.code())
.field("storage", &self.storage_changes.iter().collect::<BTreeMap<_, _>>())
.field("storage", &self.storage_changes.iter().cloned().collect::<BTreeMap<_, _>>())
.finish()
}
}
Expand Down Expand Up @@ -614,7 +620,7 @@ mod tests {

#[test]
fn rlpio() {
let a = Account::new(69u8.into(), 0u8.into(), HashMap::new(), Bytes::new());
let a = Account::new(69u8.into(), 0u8.into(), im::HashMap::new(), Bytes::new());
let b = Account::from_rlp(&a.rlp()).unwrap();
assert_eq!(a.balance(), b.balance());
assert_eq!(a.nonce(), b.nonce());
Expand All @@ -624,7 +630,7 @@ mod tests {

#[test]
fn new_account() {
let a = Account::new(69u8.into(), 0u8.into(), HashMap::new(), Bytes::new());
let a = Account::new(69u8.into(), 0u8.into(), im::HashMap::new(), Bytes::new());
assert_eq!(a.rlp().to_hex(), "f8448045a056e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421a0c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470");
assert_eq!(*a.balance(), 69u8.into());
assert_eq!(*a.nonce(), 0u8.into());
Expand All @@ -634,7 +640,7 @@ mod tests {

#[test]
fn create_account() {
let a = Account::new(69u8.into(), 0u8.into(), HashMap::new(), Bytes::new());
let a = Account::new(69u8.into(), 0u8.into(), im::HashMap::new(), Bytes::new());
assert_eq!(a.rlp().to_hex(), "f8448045a056e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421a0c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470");
}

Expand Down
5 changes: 3 additions & 2 deletions ethcore/src/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ use factory::VmFactory;
use ethereum_types::{H256, U256, Address};
use hashdb::{HashDB, AsHashDB};
use keccak_hasher::KeccakHasher;
use im;
use kvdb::DBValue;
use bytes::Bytes;

Expand Down Expand Up @@ -481,7 +482,7 @@ impl<B: Backend> State<B> {
}

/// Destroy the current object and return single account data.
pub fn into_account(self, account: &Address) -> TrieResult<(Option<Arc<Bytes>>, HashMap<H256, H256>)> {
pub fn into_account(self, account: &Address) -> TrieResult<(Option<Arc<Bytes>>, im::HashMap<H256, H256>)> {
// TODO: deconstruct without cloning.
let account = self.require(account, true)?;
Ok((account.code().clone(), account.storage_changes().clone()))
Expand Down Expand Up @@ -1052,7 +1053,7 @@ impl<B: Backend> State<B> {
}

/// Replace account code and storage. Creates account if it does not exist.
pub fn patch_account(&self, a: &Address, code: Arc<Bytes>, storage: HashMap<H256, H256>) -> TrieResult<()> {
pub fn patch_account(&self, a: &Address, code: Arc<Bytes>, storage: im::HashMap<H256, H256>) -> TrieResult<()> {
Ok(self.require(a, false)?.reset_code_and_storage(code, storage))
}
}
Expand Down