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

Add benchmark for transaction execution #11509

Merged
merged 3 commits into from
Feb 25, 2020

Conversation

dvdplm
Copy link
Collaborator

@dvdplm dvdplm commented Feb 22, 2020

Benchmark the apply method of executive-state using the transactions from two blocks.

I backported this to 2.5.13. Results:

apply transactions with tracing (Constantinople)
                        time:   [1.7979 ms 1.8201 ms 1.8432 ms]
                        change: [-4.2508% -2.4957% -0.6904%] (p = 0.01 < 0.05)
                        Change within noise threshold.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe

Benchmarking apply transactions without tracing (Constantinople): Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 9.1s or reduce sample count to 50.
apply transactions without tracing (Constantinople)
                        time:   [1.7296 ms 1.7594 ms 1.7946 ms]
                        change: [-3.6421% -0.8037% +1.7974%] (p = 0.58 > 0.05)
                        No change in performance detected.
Found 9 outliers among 100 measurements (9.00%)
  7 (7.00%) high mild
  2 (2.00%) high severe

Benchmarking apply transactions with tracing (Istanbul): Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 5.6s or reduce sample count to 60.
apply transactions with tracing (Istanbul)
                        time:   [1.0682 ms 1.0811 ms 1.0952 ms]
                        change: [-0.1363% +2.5404% +5.4194%] (p = 0.08 > 0.05)
                        No change in performance detected.
Found 12 outliers among 100 measurements (12.00%)
  7 (7.00%) high mild
  5 (5.00%) high severe

Benchmarking apply transactions without tracing (Istanbul): Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 5.4s or reduce sample count to 60.
apply transactions without tracing (Istanbul)
                        time:   [1.0307 ms 1.0421 ms 1.0557 ms]
                        change: [-12.323% -8.7760% -5.2053%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  8 (8.00%) high mild
  1 (1.00%) high severe

Current master:

apply transactions with tracing (Constantinople)
                        time:   [2.0166 ms 2.0470 ms 2.0807 ms]
                        change: [-1.1628% +1.1355% +3.8393%] (p = 0.38 > 0.05)
                        No change in performance detected.
Found 12 outliers among 100 measurements (12.00%)
  12 (12.00%) high mild

Benchmarking apply transactions without tracing (Constantinople): Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 9.7s or reduce sample count to 50.
apply transactions without tracing (Constantinople)
                        time:   [1.8491 ms 1.8758 ms 1.9056 ms]
                        change: [-3.5325% -1.0900% +1.4900%] (p = 0.40 > 0.05)
                        No change in performance detected.
Found 10 outliers among 100 measurements (10.00%)
  6 (6.00%) high mild
  4 (4.00%) high severe

Benchmarking apply transactions with tracing (Istanbul): Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 6.4s or reduce sample count to 60.
apply transactions with tracing (Istanbul)
                        time:   [1.2220 ms 1.2451 ms 1.2708 ms]
                        change: [-6.6992% -3.7312% -0.7610%] (p = 0.02 < 0.05)
                        Change within noise threshold.
Found 8 outliers among 100 measurements (8.00%)
  6 (6.00%) high mild
  2 (2.00%) high severe

Benchmarking apply transactions without tracing (Istanbul): Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 5.7s or reduce sample count to 60.
apply transactions without tracing (Istanbul)
                        time:   [1.0961 ms 1.1153 ms 1.1382 ms]
                        change: [-1.6498% +0.7117% +3.0664%] (p = 0.57 > 0.05)
                        No change in performance detected.
Found 14 outliers among 100 measurements (14.00%)
  6 (6.00%) high mild
  8 (8.00%) high severe

@dvdplm dvdplm self-assigned this Feb 22, 2020
@dvdplm dvdplm added the A0-pleasereview 🤓 Pull request needs code review. label Feb 22, 2020
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| {
Copy link
Collaborator

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?

Copy link
Collaborator Author

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?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, sorry.

Copy link
Collaborator

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Better now?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks great, thanks!

Copy link
Collaborator

@niklasad1 niklasad1 left a 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

Copy link
Collaborator

@ordian ordian left a 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
Copy link
Collaborator

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| {
Copy link
Collaborator

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

@dvdplm dvdplm mentioned this pull request Feb 24, 2020
@ordian ordian added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 25, 2020
@dvdplm dvdplm merged commit ad56eb4 into master Feb 25, 2020
@dvdplm dvdplm deleted the dp/chore/add-benchmarks-for-transaction-application branch February 25, 2020 10:25
dvdplm added a commit that referenced this pull request Feb 25, 2020
* master:
  Add benchmark for transaction execution (#11509)
  Add Smart Contract License v1.0
ordian added a commit that referenced this pull request Mar 6, 2020
* 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)
  ...
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.

None yet

3 participants