Repository: paritytech/substrate
File: utils/frame/benchmarking-cli/src/storage/write.rs (Rust)

Code before:
		&self,
		client: Arc<C>,
		(db, state_col): (Arc<dyn sp_database::Database<DbHash>>, ColumnId),
		storage: Arc<dyn sp_state_machine::Storage<HashFor<Block>>>,
	) -> Result<BenchRecord>
	where
		Block: BlockT<Header = H, Hash = DbHash> + Debug,
		H: HeaderT<Hash = DbHash>,
		BA: ClientBackend<Block>,
		C: UsageProvider<Block> + HeaderBackend<Block> + StorageProvider<Block, BA>,
	{
		// Store the time that it took to write each value.
		let mut record = BenchRecord::default();

		let block = BlockId::Number(client.usage_info().chain.best_number);
		let header = client.header(block)?.ok_or("Header not found")?;
		let original_root = *header.state_root();
		let trie = DbState::<Block>::new(storage.clone(), original_root);

		info!("Preparing keys from block {}", block);
		// Load all KV pairs and randomly shuffle them.
		let mut kvs = trie.pairs();
		let (mut rng, _) = new_rng(None);
		kvs.shuffle(&mut rng);

		let mut child_tries = Vec::new();

		// Generate all random values first; Make sure there are no collisions with existing
		// db entries, so we can rollback all additions without corrupting existing entries.
		for (k, original_v) in kvs.iter_mut() {
			if self.params.include_child && k.starts_with(DEFAULT_CHILD_STORAGE_KEY_PREFIX) {
				child_tries.push(k.clone());
			}

			'retry: loop {
				let mut new_v = vec![0; original_v.len()];
				// Create a random value to overwrite with.
				// NOTE: We use a possibly higher entropy than the original value,
				// could be improved but acts as an over-estimation which is fine for now.
				rng.fill_bytes(&mut new_v[..]);
				match check_new_value::<Block>(
					db.clone(),
					&trie,
					&k.to_vec(),
					&new_v,
					self.state_version(),
					state_col,
					None,
				) {
					true => {
						*original_v = new_v;

Review comment (suggestion):
It is currently not shuffling the keys. I am not sure how big of a problem that is, but the DB could have some caching in place that gets used if the keys are all read in order.  
Further we have to benchmark for the case that they are read in random order.  
I think its possible to write the code in a way that you first collect all the child keys as well and then shuffle and do the benchmarks, same as it was done for the top keys.

Code after:
		&self,
		client: Arc<C>,
		(db, state_col): (Arc<dyn sp_database::Database<DbHash>>, ColumnId),
		storage: Arc<dyn sp_state_machine::Storage<HashFor<Block>>>,
	) -> Result<BenchRecord>
	where
		Block: BlockT<Header = H, Hash = DbHash> + Debug,
		H: HeaderT<Hash = DbHash>,
		BA: ClientBackend<Block>,
		C: UsageProvider<Block> + HeaderBackend<Block> + StorageProvider<Block, BA>,
	{
		// Store the time that it took to write each value.
		let mut record = BenchRecord::default();

		let block = BlockId::Number(client.usage_info().chain.best_number);
		let header = client.header(block)?.ok_or("Header not found")?;
		let original_root = *header.state_root();
		let trie = DbState::<Block>::new(storage.clone(), original_root);

		info!("Preparing keys from block {}", block);
		// Load all KV pairs and randomly shuffle them.
		let mut kvs = trie.pairs();
		let (mut rng, _) = new_rng(None);
		kvs.shuffle(&mut rng);
		info!("Writing {} keys", kvs.len());

		let mut child_nodes = Vec::new();

		// Generate all random values first; Make sure there are no collisions with existing
		// db entries, so we can rollback all additions without corrupting existing entries.
		for (k, original_v) in kvs {
			match (self.params.include_child_trees, self.is_child_key(k.to_vec())) {
				(true, Some(info)) => {
					let child_keys =
						client.child_storage_keys_iter(&block, info.clone(), None, None)?;
					for ck in child_keys {
						child_nodes.push((ck.clone(), info.clone()));
					}
				},
				_ => {
					// regular key
					let mut new_v = vec![0; original_v.len()];
					loop {
						// Create a random value to overwrite with.
						// NOTE: We use a possibly higher entropy than the original value,
						// could be improved but acts as an over-estimation which is fine for now.
						rng.fill_bytes(&mut new_v[..]);
						if check_new_value::<Block>(
							db.clone(),
							&trie,
							&k.to_vec(),