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

Trie query recording and AccountDB factory for no mangling #1944

Merged
merged 8 commits into from
Aug 24, 2016

Conversation

rphmeier
Copy link
Contributor

I thought it was important to have the recorder stuff be statically dispatched so LLVM can inline out record calls to NoOp recorders. This lead to TrieDB not being object-safe any more so I put together an enum that the trie factory produces, which has the added bonus of reducing allocations and virtual dispatch for trie queries. Probably will extend that to TrieMut as well.

@rphmeier rphmeier added the A0-pleasereview 🤓 Pull request needs code review. label Aug 17, 2016
@rphmeier rphmeier changed the title Trie query recording Trie query recording and AccountDB factory for no mangling Aug 17, 2016
@rphmeier rphmeier force-pushed the trie_record branch 2 times, most recently from 0b0a669 to 9b993d7 Compare August 17, 2016 16:08
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 87.059% when pulling 2792759 on trie_record into bcf6b0b on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 87.056% when pulling 2792759 on trie_record into bcf6b0b on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 86.96% when pulling 5d34914 on trie_record into 0620a03 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 86.918% when pulling 6ee70ed on trie_record into 0e0cc20 on master.

@@ -51,10 +51,12 @@ impl StateProducer {
// modify existing accounts.
let mut accounts_to_modify: Vec<_> = {
let trie = TrieDB::new(&*db, &self.state_root).unwrap();
trie.iter()
let regionck_bug = trie.iter()
Copy link
Contributor

Choose a reason for hiding this comment

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

???

Copy link
Contributor Author

@rphmeier rphmeier Aug 23, 2016

Choose a reason for hiding this comment

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

knee-jerk reaction to working around a lifetime checker (regionck) error in rustc

Copy link
Contributor

Choose a reason for hiding this comment

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

right - figured it might be something like that. shame it's buggy. if there's a tracking issue in the rust GH would be good to note it so can be fixed later.

@gavofyork
Copy link
Contributor

a decent chunk of tests would be nice.

@gavofyork gavofyork added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 23, 2016
@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Aug 24, 2016
@rphmeier
Copy link
Contributor Author

The recorders are unit-tested, and I've just added a test which verifies the recording for a given trie.

The next PR will create a more generic "prover" module which will create pseudorandom tries and verify proofs as well as idempotence.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 86.858% when pulling f0e94c6 on trie_record into d4777f9 on master.

@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 24, 2016
gavofyork pushed a commit that referenced this pull request Aug 24, 2016
* optionally use no mangling for accountdb

* add the recorder module

* get_recorded for tries, no virtual dispatch on readonly tries

* add recording test
@gavofyork gavofyork merged commit 190e4db into master Aug 24, 2016
@gavofyork gavofyork deleted the trie_record branch August 24, 2016 14:53
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants