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

refactor: improve benchmark compile output #445

Merged
merged 1 commit into from
Dec 16, 2021

Conversation

secretfader
Copy link

Currently, several of our benchmarks output warnings concerning unused imports in addition to the benchmark data we requested. This is unaffected by the --quiet flag, and in at least one case, we may be compiling more dependency code than necessary.

This PR resolves the above issues through use of Rust compiler flags.

@secretfader secretfader changed the title WIP: improve benchmark compile output Improve benchmark compile output Dec 13, 2021
@secretfader secretfader marked this pull request as draft December 13, 2021 19:36
@codecov
Copy link

codecov bot commented Dec 13, 2021

Codecov Report

Merging #445 (f4d5b1e) into master (522a04f) will not change coverage.
The diff coverage is n/a.

❗ Current head f4d5b1e differs from pull request most recent head 68f0e79. Consider uploading reports for the commit 68f0e79 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master     #445   +/-   ##
=======================================
  Coverage   75.15%   75.15%           
=======================================
  Files          83       83           
  Lines        9800     9800           
=======================================
  Hits         7365     7365           
  Misses       2435     2435           
Impacted Files Coverage Δ
pallets/allocations/src/benchmarking.rs 100.00% <ø> (ø)
pallets/allocations/src/lib.rs 82.97% <ø> (ø)
pallets/amendments/src/benchmarking.rs 100.00% <ø> (ø)
pallets/emergency-shutdown/src/benchmarking.rs 100.00% <ø> (ø)
pallets/grants/src/benchmarking.rs 100.00% <ø> (ø)
pallets/reserve/src/benchmarking.rs 100.00% <ø> (ø)
pallets/root-of-trust/src/benchmarking.rs 100.00% <ø> (ø)
pallets/staking/src/benchmarking.rs 95.31% <ø> (ø)
pallets/tcr/src/benchmarking.rs 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 522a04f...68f0e79. Read the comment docs.

@secretfader secretfader force-pushed the refactor/remove-unused-import-benchmark branch from 2358947 to b02c4a6 Compare December 13, 2021 23:23
@secretfader secretfader marked this pull request as ready for review December 13, 2021 23:24
@secretfader secretfader changed the title Improve benchmark compile output refactor: improve benchmark compile output Dec 13, 2021
@secretfader
Copy link
Author

This branch should be executed with the following command and return no errors in a clean environment.

cargo run --release --features runtime-benchmarks --manifest-path=node/Cargo.toml -- benchmark --pallet="frame_system" --extrinsic="*"

Note the inclusion of --release; this ensures we're not compiling debug or test code (which should make the runs quicker), and enable production optimizations (to place our results closer to real-world conditions).

ETeissonniere
ETeissonniere previously approved these changes Dec 14, 2021
@secretfader secretfader force-pushed the refactor/remove-unused-import-benchmark branch from f4d5b1e to 68f0e79 Compare December 14, 2021 19:55
@secretfader
Copy link
Author

I resolved the errors when running cargo test --all --all-features by allowing unused imports in the benchmarking module specifically.

I'll merge after the test run passes.

@secretfader secretfader merged commit 38f24cf into master Dec 16, 2021
@secretfader secretfader deleted the refactor/remove-unused-import-benchmark branch December 16, 2021 23:52
ETeissonniere added a commit that referenced this pull request Jan 20, 2022
* make the allocate extrinsic free for the oracles (#418)

* make the allocate extrinsic free for the oracles

* bump version numbers

* add staking-local spec (#419)

* update decimals (#421)

* split runtimes (#420)

* move and rename runtime to one subfolder

* remove unused deps on pallet-staking

* remove old migrations code

* create minimum staking runtime

* remove with-staking feature flag

* remove unused spec

* remove executor crate

* remove nodle_ and nodle_chain_ prefixes

* make node work with runtime_main

* fix benchmarking

* fix srtool config (#426)

* fix folder

* fix package name

* freeze rust version

* rename artifact

* fix github action issues

* master only

* bump version for release

* upgrade to polkadot-v0.9.12 (#424)

* upgrade to polkadot-v0.9.12

Signed-off-by: R.RajeshKumar <[email protected]>

* review comments fix

Signed-off-by: R.RajeshKumar <[email protected]>

* fmt fix

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow bringup

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow bringup

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow bringup

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow bringup

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow bringup

Signed-off-by: R.RajeshKumar <[email protected]>

* Review changes

* Staking migration updates
* Addition of rust-toolchain.toml

* ci workflow bringup

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow fix

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow fix

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow fix

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow fix

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow fix

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow fix

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow fix

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow fix

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow fix

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow fix

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow fix

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow fix

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow fix

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow fix

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow fix

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow fix

Signed-off-by: R.RajeshKumar <[email protected]>

* ci workflow fix

Signed-off-by: R.RajeshKumar <[email protected]>

* Bump serde_json from 1.0.69 to 1.0.70 (#428)

Bumps [serde_json](https://github.com/serde-rs/json) from 1.0.69 to 1.0.70.
- [Release notes](https://github.com/serde-rs/json/releases)
- [Commits](serde-rs/json@v1.0.69...v1.0.70)

---
updated-dependencies:
- dependency-name: serde_json
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump serde_json from 1.0.70 to 1.0.71 (#429)

* Bump actions/cache from 2.1.6 to 2.1.7 (#431)

Bumps [actions/cache](https://github.com/actions/cache) from 2.1.6 to 2.1.7.
- [Release notes](https://github.com/actions/cache/releases)
- [Commits](actions/cache@v2.1.6...v2.1.7)

---
updated-dependencies:
- dependency-name: actions/cache
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump futures from 0.3.17 to 0.3.18 (#432)

Bumps [futures](https://github.com/rust-lang/futures-rs) from 0.3.17 to 0.3.18.
- [Release notes](https://github.com/rust-lang/futures-rs/releases)
- [Changelog](https://github.com/rust-lang/futures-rs/blob/master/CHANGELOG.md)
- [Commits](rust-lang/futures-rs@0.3.17...0.3.18)

---
updated-dependencies:
- dependency-name: futures
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* runtime main & staking integration to node (#427)

* runtime main & staking integration to node

Signed-off-by: R.RajeshKumar <[email protected]>

* node rpc refactor

Signed-off-by: R.RajeshKumar <[email protected]>

* change in runtime detect logic

Signed-off-by: R.RajeshKumar <[email protected]>

* Review comments closure

Signed-off-by: R.RajeshKumar <[email protected]>

* cargo fmt

Signed-off-by: R.RajeshKumar <[email protected]>

* nodle-chain version correction

Signed-off-by: R.RajeshKumar <[email protected]>

* bump version for release

* fix release script

* nodle parachain bringup (#433)

* also log into docker hub (#435)

* also log into docker hub

* rename workflow

* pallet grants BlockNumberProvider integration (#436)

Signed-off-by: R.RajeshKumar <[email protected]>

* minimize chain spec parameters (#437)

* create eden-local chain spec (#438)

* fix spec import (#440)

* Enable endowed accounts (#441)

* production eden spec (#442)

* generalize session key injection to aura and collator

* no longer inject session keys via curl

* kill chaos config

* move dockerfile up

* minimum production chain spec

* mount chain spec in node

* fine tune chain spec

* add placeholder bootnode

* add call filter

* bump version for release

* generate compressed runtime as well

* fix parachain id in genesis config

* remove log

* Bump serde from 1.0.130 to 1.0.131 (#443)

* pallets: reorganize benchmark compilation flags (#445)

* feat: support try-runtime for all our runtimes (#444)

* Bump serde from 1.0.131 to 1.0.132 (#446)

Bumps [serde](https://github.com/serde-rs/serde) from 1.0.131 to 1.0.132.
- [Release notes](https://github.com/serde-rs/serde/releases)
- [Commits](serde-rs/serde@v1.0.131...v1.0.132)

---
updated-dependencies:
- dependency-name: serde
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump futures from 0.3.18 to 0.3.19 (#447)

Bumps [futures](https://github.com/rust-lang/futures-rs) from 0.3.18 to 0.3.19.
- [Release notes](https://github.com/rust-lang/futures-rs/releases)
- [Changelog](https://github.com/rust-lang/futures-rs/blob/master/CHANGELOG.md)
- [Commits](rust-lang/futures-rs@0.3.18...0.3.19)

---
updated-dependencies:
- dependency-name: futures
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump serde from 1.0.132 to 1.0.133 (#448)

Bumps [serde](https://github.com/serde-rs/serde) from 1.0.132 to 1.0.133.
- [Release notes](https://github.com/serde-rs/serde/releases)
- [Commits](serde-rs/serde@v1.0.132...v1.0.133)

---
updated-dependencies:
- dependency-name: serde
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feat(pallet-allocations): fast return on zero allocation (#450)

* feat: fast return on zero allocation

* test: fast return on zero allocation

* fix(ci): remove dead code (#451)

Installing ubuntu native dependencies had become dead as of a previous process simplification. It turns out CI runs fine without it.

* Upgrade rust edition (#449)

* fix: rust edition to 2021

* feat: update to polkadot v0.9.13 to fix ChainSpecExtention macro issue

* fix: remove light-client support

light-client is experiencing major change in the upstream. It's also not supported for para-chains yet

* fix: wiring of the new slash parameter disable_strategy

The parameter is not yet consumed properly in our slash computation. The proper support will come in NodleCode/chain-workspace#95

* fix: resolving para-id from cli. Use default para-id (1000) with "eden-local" and "eden-dev".

The id/para-id is not an explicit run command for a parachain anymore. It's assumed the parachain id is part of the extended chain spec. So for example no need to specify both "eden" and its id. "eden" should have a known id.

* fix(ci): specify the main-net wss port explicitly.

* minor refactorings (#454)

* Staking proto chainspec & verification updates ... (#453)

* staking proto chainspec & verification updates

Signed-off-by: shamb0 <[email protected]>

* cargo fmt fix

Signed-off-by: shamb0 <[email protected]>

* review fix

Signed-off-by: shamb0 <[email protected]>

* review fix

Signed-off-by: rajesh-nodle <[email protected]>

Co-authored-by: shamb0 <[email protected]>

* update copyright year from 2020 to 2022 (#460)

* preinstall toolchain in devcontainer (#473)

* replace arcadia chain spec (#474)

Co-authored-by: rajesh-nodle <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Alex Sedighi <[email protected]>
Co-authored-by: Nicholas Young <[email protected]>
Co-authored-by: shamb0 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants