Add DB Read/Write Tracking to Benchmarking Pipeline

[shawntabrizi]

This PR is the first step toward adding full automation to the benchmarking pipeline.

Here, we modify the Bench DB to add an additional HashMap to track keys that have been read from and written to.

We are then able to run out bechmarks, and inspect this read/write tracker to see the number of times we do a read, repeat read, write, or repeat write.

This information is then stored as part of our BenchmarkResults output and displayed as part of the CLI.

Next steps for this is:

• Add a whitelist of keys to ignore for counting
• Add regression analysis to the reads/writes
• Output to Rust module

[shawntabrizi]

Here is an example of the output:

➜  cli git:(shawntabrizi-bench-db-tracking) ✗ ../../../target/debug/substrate benchmark --pallet balances --extrinsic transfer --raw
2020-06-18 12:48:11 💸 new validator set of size 1 has been elected via ElectionCompute::OnChain for era 0
Pallet: "balances", Extrinsic: "transfer", Lowest values: [], Highest values: [], Steps: [], Repeat: 1
u,e,extrinsic_time,storage_root_time,reads,repeat_reads,writes,repeat_writes
1,1000,724000,53000,7,10,5,0
100,1000,715000,49000,7,10,5,0
199,1000,745000,33000,7,10,5,0
298,1000,743000,35000,7,10,5,0
397,1000,743000,33000,7,10,5,0
496,1000,790000,31000,7,10,5,0
595,1000,650000,29000,7,10,5,0
694,1000,595000,27000,7,10,5,0
793,1000,593000,28000,7,10,5,0
892,1000,608000,28000,7,10,5,0
991,1000,610000,28000,7,10,5,0
1000,2,600000,101000,7,10,5,0
1000,101,602000,28000,7,10,5,0
1000,200,603000,28000,7,10,5,0
1000,299,610000,28000,7,10,5,0
1000,398,613000,27000,7,10,5,0
1000,497,604000,29000,7,10,5,0
1000,596,604000,28000,7,10,5,0
1000,695,611000,28000,7,10,5,0
1000,794,612000,29000,7,10,5,0
1000,893,648000,27000,7,10,5,0
1000,992,604000,28000,7,10,5,0

[kianenigma]

Looks good to me, I'll try and do another review later.

This means that we don't have to do that manual code analysis counting reads and writes right? I am curious to just see how off we were before 😁

[shawntabrizi]

@kianenigma This fixes the static read/write counting we were doing for extrinsics. It should give us the data so that we can create a weight formula with a "maximum" number of reads/writes the extrinsic will do.

But it does not fix the on_initialize/on_finalize counting that we are doing and still need to do.

[shawntabrizi]

Could I also get a review on: https://github.com/paritytech/substrate/pull/6405/files

This is a PR on top of this PR to add whitelisting to to the DB Read/Write tracking

[kianenigma]

correct me if I am wrong: by whitelisting, basically we already assume that they have been read once? there's no inherent meaning to being whitelisted other than that.

[shawntabrizi]

Yes, this key is free to read and write to, and does not count in DB tracking.

Things like BlockNumber, the Sender Account, Events, etc...

[kianenigma]

Okay, one question then: won't this cause confusion with the read-write count? Say I submit a tx that has no reads and writes. Won't the read/write count of my tx then be equal to all the whitelisted ones?

[kianenigma]

(Either way I think it is all okay, I am just trying to make it clear for myself.)

[shawntabrizi]

no, I dont increment the read/write count when I do whitelisting. So assuming nothing is actually read/written from, you will get 0 reads and 0 writes.

[kianenigma]

this logs also in wasm, which is probably not desirable I think?

Suggested change

	frame_support::debug::trace!(target: "benchmark", "Read/Write Count {:?}", read_write_count);
	frame_support::debug::native::trace!(target: "benchmark", "Read/Write Count {:?}", read_write_count);

[shawntabrizi]

I also want these logs to emit during wasm execution, which is the standard way to run these benchmarks. Per conversation in "dumb questions", this should have no overhead when the log flag is not included

[kianenigma]

I gave this another read and still positive about it.

[shawntabrizi]

@kianenigma i agree, I would like to keep it separate if possible, and start merging in this working first step

[shawntabrizi]

For anyone reviewing, line width error here I think is unavoidable (caused by string literal in hex! macro.