-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add benchmark for transaction execution #11509
Add benchmark for transaction execution #11509
Conversation
let signed_txs = setup_state_for_block(&mut state, constantinople_block); | ||
let machine = new_constantinople_test_machine(); | ||
|
||
c.bench_function("apply transactions with tracing (Constantinople)", |b| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the benches seem very similar with the exception that the boolean flag is_checkpoint
differs. Is it possible to extract those to a helper function to avoid repeating?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, is_checkpoint
? Do you mean the tracing
bool?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, sorry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for extracting the logic that is repeated 4 times
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, some nitpicks from myside
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just small nits.
// You should have received a copy of the GNU General Public License | ||
// along with Parity Ethereum. If not, see <http://www.gnu.org/licenses/>. | ||
|
||
//! Benchmark transaction execution |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tricky part is that the transaction execution depends on the db size, here we're measuring only CPU time as if all the data fits in memory. I think it's worth mentioning in the module docs.
let signed_txs = setup_state_for_block(&mut state, constantinople_block); | ||
let machine = new_constantinople_test_machine(); | ||
|
||
c.bench_function("apply transactions with tracing (Constantinople)", |b| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for extracting the logic that is repeated 4 times
…oth Constantinople and Istanbul rules
* master: Add benchmark for transaction execution (#11509) Add Smart Contract License v1.0
* master: (27 commits) Faster kill_garbage (#11514) [EngineSigner]: don't sign message with only zeroes (#11524) fix compilation warnings (#11522) [ethcore cleanup]: various unrelated fixes from `#11493` (#11507) Add benchmark for transaction execution (#11509) Add Smart Contract License v1.0 Misc fixes (#11510) [dependencies]: unify `rustc-hex` (#11506) Activate on-chain randomness in POA Sokol (#11505) Grab bag of cleanup (#11504) Implement eth/64 (EIP-2364) and drop support for eth/62 (#11472) [dependencies]: remove `util/macros` (#11501) OpenEthereum bootnodes are added (#11499) [ci benches]: use `all-features` (#11496) [verification]: make test-build compile standalone (#11495) complete null-signatures removal (#11491) Include the seal when populating the header for a new block (#11475) fix compilation warnings (#11492) cargo update -p cmake (#11490) update to published rlp-derive (#11489) ...
Benchmark the
apply
method ofexecutive-state
using the transactions from two blocks.I backported this to 2.5.13. Results:
Current master: