Stateful storage pov optimization

[aramikm]

Goal

The goal of this PR is to evaluate and minimize PoV consumption by stateful-storage pallet.

Closes #1782

Discussion

• refactored benchmarks to calculate the max of time and PoV
• decreased additional-trie-layers number
• decreased MaxItemizedPageSizeBytes from 64KiB to around 10KiB (will allow around 292 of 32 bytes itemized public keys)

Improvements

• by applying mentioned changes the PoV for apply_item_actions got reduced from 45KB to 15KB
• by applying mentioned changes the PoV for upsert and delete pages got reduced from 12KB to 6KB

Future improvements

• by using PoV clawback we can further reduce the PoV sizes. A PR to track

Checklist

• Benchmarks added
• Weights updated

[github-actions]

⏳ Running benchmarks and calculating weights. DO NOT MERGE! A new commit will be added upon completion...

[github-actions]

⏳ Running benchmarks and calculating weights. DO NOT MERGE! A new commit will be added upon completion...

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (36c4e4b) 87.62% compared to head (610bff8) 87.60%.

❗ Current head 610bff8 differs from pull request most recent head 64eb8a6. Consider uploading reports for the commit 64eb8a6 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1757      +/-   ##
==========================================
- Coverage   87.62%   87.60%   -0.02%     
==========================================
  Files          52       52              
  Lines        4346     4339       -7     
==========================================
- Hits         3808     3801       -7     
  Misses        538      538              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

✅ Finished running benchmarks. Updated weights have been committed to this PR branch in commit 18b7a8d.

[github-actions]

⏳ Running benchmarks and calculating weights. DO NOT MERGE! A new commit will be added upon completion...

[github-actions]

⏳ Running benchmarks and calculating weights. DO NOT MERGE! A new commit will be added upon completion...

[github-actions]

✅ Finished running benchmarks. Updated weights have been committed to this PR branch in commit 7994714.

[JoeCap08055]

Looks pretty good. A couple of questions:

• Do we really want to run all benchmarks? Seems like we could omit unrelated benchmark files.
• Am I reading it wrong, or did some of the weights in stateful-storage increase?

[aramikm]

Looks pretty good. A couple of questions:

• Do we really want to run all benchmarks? Seems like we could omit unrelated benchmark files.

Yes we do since changing the --additional-trie-layers to 5 decreases PoV for all pallets across the board

• Am I reading it wrong, or did some of the weights in stateful-storage increase?

So PoV for all of them should be decreased.
Since the underlying logic is the same the ref-times should not have a significant change. Other than that I improved benchmarks for itemized related extrinsics to be more accurate.

Please let me know which one you think increased more than regular benchmark fluctuations.

[shannonwells]

Non-blocking comments only -

I did not look through all of the weights files but I did look through about a dozen and yes it seems that those changes are within the normal fluctuation of actual weights and error margins, whereas PoV is dramatically reduced. I do wonder about the child trie estimate though, how did you come up with the estimate of 1M child tries and why do you think that is a good estimate?

Is there a drawback of giving enough ceiling for 10M tries i.e. would it be too costly to overestimate the PoV, so does it make more sense to increase only when needed?

[aramikm]

I did not look through all of the weights files but I did look through about a dozen and yes it seems that those changes are within the normal fluctuation of actual weights and error margins, whereas PoV is dramatically reduced. I do wonder about the child trie estimate though, how did you come up with the estimate of 1M child tries and why do you think that is a good estimate?

The idea is that since child tries are different from regular StorageMaps in the sense that StorageMaps are bottom-heavy. Meaning, the number of nodes from root of the tree to the root of the storage is usually less than the number of nodes between the root of the storage to the last inserted node in map on average (Considering we have a map with around at least 1 million items). In child tries the path from root of the tree to the root of a child trie would require more node traversal compared to the path from root of child trie to a member (Considering each child trie's inserted items are low).
In another words for regular StorageMaps, step # 1 of PoV calculation mentioned in one of comments are much smaller compared to step # 2 but this is not the case for child tries.

Having the above explanation to make sure our PoV calculations are considering this fact we need to ensure that step # 1 of PoV calculations are going to consider the max number of existing child tries for benchmarks. Since on a balanced tree in substrate we will need to access Log16(X) nodes for X inserted items, if we consider having 1_000_000 child tries for benchmarks we will get Log16(1_000_000) ~ 5

Is there a drawback of giving enough ceiling for 10M tries i.e. would it be too costly to overestimate the PoV, so does it make more sense to increase only when needed?

A number of things to consider

• it would be less efficient, a +1 change in that parameter would increase all PoVs in all pallets by around +600 bytes.
• We would probably need to only isolate this --additional-trie-layers = 5 or more for only benchmarks running for stateful-storage in future as an optimization. It doesn't need to affect all pallets.
• This is my current understanding of how everything work based on a number of experiments. To 100% make sure how they handle (or don't handle) child tires we will need to dive deep into the PoV calculation code.

[aramikm]

1 Million child trees was a guesstimate based on the number of users on chain. Considering we would need to update our benchmark params every 4-6 months if we reached that point can bump it along with other benchmark calculations like database access benchmarks and etc which should be based on the data inside merkle tree.

[JoeCap08055]

• Am I reading it wrong, or did some of the weights in stateful-storage increase?

So PoV for all of them should be decreased. Since the underlying logic is the same the ref-times should not have a significant change. Other than that I improved benchmarks for itemized related extrinsics to be more accurate.

Please let me know which one you think increased more than regular benchmark fluctuations.

OK, looking again it was just some of the reference times that increased, which is probably normal fluctuation. PoVs decreased significantly 👍🏻