Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PoV Reclaim (Clawback) Node Side #1462

Merged
merged 80 commits into from
Nov 30, 2023
Merged

Conversation

skunert
Copy link
Contributor

@skunert skunert commented Sep 8, 2023

This PR provides the infrastructure for the pov-reclaim mechanism discussed in #209. The goal is to provide the current proof size to the runtime so it can be used to reclaim storage weight.

New Host Function

  • A new host function is provided here. It returns the size of the current proof size to the runtime. If recording is not enabled, it returns 0.

Implementation Overview

  • Implement option to enable proof recording during import in the client. This is currently enabled for polkadot-parachain, parachain-template and the cumulus test node.
  • Make the proof recorder ready for no-std. It was previously only enabled for std environments, but we need to record the proof size in validate_block too.
  • Provide a recorder implementation that only the records the size of incoming nodes and does not store the nodes itself.
  • Fix benchmarks that were broken by async backing changes
  • Provide new externalities extension that is registered by default if proof recording is enabled.
  • I think we should discuss the naming, pov-reclaim was more intuitive to me, but we could also go with clawback like in the issue.

Impact of proof recording during import

With proof recording: 6.3058 Kelem/s
Without proof recording: 6.3427 Kelem/s

The measured impact on the importing performance is quite low on my machine using the block import benchmark. With proof recording I am seeing a performance hit of 0.585%.

@skunert
Copy link
Contributor Author

skunert commented Sep 8, 2023

bot fmt

@command-bot
Copy link

command-bot bot commented Sep 8, 2023

@skunert https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3618768 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 1-44209f3d-8a92-4716-a9cc-28baa16984a0 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Nov 1, 2023

@skunert Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/4162396 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/4162396/artifacts/download.

@skunert
Copy link
Contributor Author

skunert commented Nov 2, 2023

I removed usages of the host function in c4b0e05, since it was blocking CI. We use try-runtime in CI which does not know this hf yet and is therefore crashing.

However, I will add the contents of that commit to the follow-up PR. From my point of view this is now ready to merge. I will wait until next week if something comes up in the RFC, otherwise merge.

aramikm added a commit to frequency-chain/frequency that referenced this pull request Nov 16, 2023
# 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](paritytech/polkadot-sdk#1462) to track

# Checklist
- [x] Benchmarks added
- [x] Weights updated

---------

Co-authored-by: Frequency CI [bot] <do-not-reply@users.noreply.github.com>
Co-authored-by: Wil Wade <wil.wade@unfinished.com>
@skunert skunert merged commit 9a650c4 into paritytech:master Nov 30, 2023
114 checks passed
paritytech-rfc-bot bot pushed a commit to polkadot-fellows/RFCs that referenced this pull request Feb 2, 2024
…lock Utilization (#43)

paritytech/polkadot-sdk#1462 has been open for a
while and is now well-reviewed. Opening an RFC since it introduces a new
host function that is relevant for parachains and light-clients.
@cheme cheme mentioned this pull request Feb 19, 2024
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/parity-tech-update-for-february/6630/1

bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
This PR provides the infrastructure for the pov-reclaim mechanism
discussed in paritytech#209. The goal is to provide the current proof size to the
runtime so it can be used to reclaim storage weight.

## New Host Function
- A new host function is provided
[here](https://github.com/skunert/polkadot-sdk/blob/5b317fda3be205f4136f10d4490387ccd4f9765d/cumulus/primitives/pov-reclaim/src/lib.rs#L23).
It returns the size of the current proof size to the runtime. If
recording is not enabled, it returns 0.

## Implementation Overview
- Implement option to enable proof recording during import in the
client. This is currently enabled for `polkadot-parachain`,
`parachain-template` and the cumulus test node.
- Make the proof recorder ready for no-std. It was previously only
enabled for std environments, but we need to record the proof size in
`validate_block` too.
- Provide a recorder implementation that only the records the size of
incoming nodes and does not store the nodes itself.
- Fix benchmarks that were broken by async backing changes
- Provide new externalities extension that is registered by default if
proof recording is enabled.
- I think we should discuss the naming, pov-reclaim was more intuitive
to me, but we could also go with clawback like in the issue.

## Impact of proof recording during import
With proof recording: 6.3058 Kelem/s
Without proof recording: 6.3427 Kelem/s

The measured impact on the importing performance is quite low on my
machine using the block import benchmark. With proof recording I am
seeing a performance hit of 0.585%.

---------

Co-authored-by: command-bot <>
Co-authored-by: Davide Galassi <davxy@datawok.net>
Co-authored-by: Bastian Köcher <git@kchr.de>
@skunert skunert deleted the pov-reclaim branch April 3, 2024 08:44
rustadot added a commit to rustadot/recurrency that referenced this pull request Sep 5, 2024
# 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](paritytech/polkadot-sdk#1462) to track

# Checklist
- [x] Benchmarks added
- [x] Weights updated

---------

Co-authored-by: Frequency CI [bot] <do-not-reply@users.noreply.github.com>
Co-authored-by: Wil Wade <wil.wade@unfinished.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T0-node This PR/Issue is related to the topic “node”. T5-host_functions This PR/Issue is related to host functions.
Projects
Status: done
Status: Audited
Development

Successfully merging this pull request may close these issues.

7 participants