contracts: Don't read the previous value when overwriting a storage item #7879
Conversation
athei
added
A0-please_review
|
/bench pallet pallet_contracts |
|
Error running benchmark: at-optimize-storage-write stdoutTypeError: Cannot read property 'title' of undefined |
|
/benchmark runtime pallet pallet_contracts |
|
Finished benchmark for branch: at-optimize-storage-write Benchmark: Benchmark Runtime Pallet cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_contracts --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/contracts/src/weights.rs --template=./.maintain/frame-weight-template.hbs ResultsDownloading crates ... |
|
/benchmark runtime pallet pallet_contracts |
|
Finished benchmark for branch: at-optimize-storage-write Benchmark: Benchmark Runtime Pallet cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_contracts --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/contracts/src/weights.rs --template=./.maintain/frame-weight-template.hbs ResultsPallet: "pallet_contracts", Extrinsic: "on_initialize", Lowest values: [], Highest values: [], Steps: [50], Repeat: 20
|
…/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_contracts --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/contracts/src/weights.rs --template=./.maintain/frame-weight-template.hbs
shawntabrizi
requested review from
cheme
and removed request for
shawntabrizi
There was a problem hiding this comment.
weights have changed a bit but it seems to be because of changes before this PR (#7879 (comment)), and new weights looks good also.
so looks good to me
|
By looking at the linked PR we see that the new storage access was not introduced by this PR. |
|
bot merge |
|
Trying merge. |
ghost
merged commit …tem (#7879) * Add `len` function that can return the length of a storage item efficiently * Make use of the new len function in contracts * Fix benchmarks * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_contracts --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/contracts/src/weights.rs --template=./.maintain/frame-weight-template.hbs * Remove unused imports Co-authored-by: Parity Benchmarking Bot <[email protected]>
* make helper error types generics * avoid From<io::Error> dep in runner helper logic * slip of the pen, bump futures to 0.3.9 * more generics * generic var spaces Co-authored-by: Andronik Ordian <[email protected]> * network-gossip: add metric for number of local messages (#7871) * network-gossip: add metric for number of local messages * grandpa: fix GossipEngine missing metrics registry parameter * network-gossip: increase known messages cache size * network-gossip: fix tests * grandpa: remove unnecessary clone Co-authored-by: Max Inden <[email protected]> * network-gossip: count registered and expired messages separately * network-gossip: add comment on known messages cache size * network-gossip: extend comment with cache size in memory Co-authored-by: Max Inden <[email protected]> * Clean-up pass in network/src/protocol.rs (#7889) * Remove statistics system * Remove ContextData struct * Remove next_request_id * Some TryFrom nit-picking * Use constants for peer sets * contracts: Don't read the previous value when overwriting a storage item (#7879) * Add `len` function that can return the length of a storage item efficiently * Make use of the new len function in contracts * Fix benchmarks * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_contracts --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/contracts/src/weights.rs --template=./.maintain/frame-weight-template.hbs * Remove unused imports Co-authored-by: Parity Benchmarking Bot <[email protected]> * Fix clear prefix check to avoid erasing child trie roots. (#7848) * Fix clear prefix check to avoid erasing child trie roots. * Renaming and extend existing test with check. * last nitpicks. * use follow paths to std standarad components * line width Co-authored-by: Bernhard Schuster <[email protected]> Co-authored-by: Andronik Ordian <[email protected]> Co-authored-by: André Silva <[email protected]> Co-authored-by: Max Inden <[email protected]> Co-authored-by: Pierre Krieger <[email protected]> Co-authored-by: Alexander Theißen <[email protected]> Co-authored-by: Parity Benchmarking Bot <[email protected]> Co-authored-by: cheme <[email protected]>
The contracts pallet tracks how much storage in bytes every contract occupies. In order to do so when overwriting the value the size of the old value needs to be determined and subtracted:
substrate/frame/contracts/src/storage.rs
Lines 124 to 127 in f977fb8
To do so the previous value was read into a
Vecand then the length of it was determined. However, reading the whole value is not necessary. This PR rectifies this behaviour by only reading the size of the storage item.