-
Notifications
You must be signed in to change notification settings - Fork 503
Measured base block & extrinsic overhead #1514
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
Conversation
ipapandinas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the following TODO be addressed now?
Astar/runtime/shibuya/src/lib.rs
Lines 254 to 259 in 0ebcf7f
| // Adjusting the base extrinsic weight to account for the additional database | |
| // read introduced by the `tx-pause` pallet during extrinsic filtering. | |
| // | |
| // TODO: This hardcoded addition is a temporary fix. Replace it with a proper | |
| // benchmark in the future. | |
| weights.base_extrinsic = ExtrinsicBaseWeight::get().saturating_add(<Runtime as frame_system::Config>::DbWeight::get().reads(1)); |
Sure, I forgot it's present in Shibuya too, and wanted to handle it in the tx-pause/safe-mode PR for Astar. |
ipapandinas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I forgot it's present in Shibuya too, and wanted to handle it in the tx-pause/safe-mode PR for Astar.
Thanks for handling it here, it could have been done in the Astar PR, I didn't want to slow down your tests.
Minimum allowed line rate is |
| // TODO: This hardcoded addition is a temporary fix. Replace it with a proper | ||
| // benchmark in the future. | ||
| weights.base_extrinsic = ExtrinsicBaseWeight::get().saturating_add(<Runtime as frame_system::Config>::DbWeight::get().reads(1)); | ||
| weights.base_extrinsic = ExtrinsicBaseWeight::get(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if BaseExtrinicWeight includes reads
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although this read will happen only once, maybe it's ok not to measure it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does measure it since filter is part of extrinsic execution.
I'll confirm this in the other PR after re-running the benchmarks for Astar.
For measurement, it doesn't happen once per block but once per extrinsic.
Tx-pause is implemented in such a way that key is (pallet name, extrinsic name).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I was thinking of safe-mode :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assumed that safe-mode read is whitelisted...seems it's not.
Pull Request Summary
Parachain runtimes now use benchmarked block & extrinsic overhead.
Also updated benchmark script not to try & generate weights in case of an error.