Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Disable logging in runtime when enabling runtime-benchmarks #2901

Closed
athei wants to merge 2 commits into from
Closed

Disable logging in runtime when enabling runtime-benchmarks #2901

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5ac8b2e
Disable logging in runtimes for runtime-benchmarking
athei Apr 19, 2021
5e71354
Merge branch 'master' into at-disable-logging-on-benchmark
athei Apr 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions runtime/common/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ std = [
"xcm/std",
]
runtime-benchmarks = [
"sp-api/disable-logging",
Copy link
Copy Markdown
Contributor

@kianenigma kianenigma Apr 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need to enable this in other places as well?

Copy link
Copy Markdown
Member Author

@athei athei Apr 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The common runtime is a dependency of all other runtimes. AFAIK this is the only place where we need to at it. It does not seem to be applicable to bridge and xcm runtimes because those don't depend on sp-api. Maybe someone else can step in with more knowledge about those runtimes.

Copy link
Copy Markdown
Contributor

@kianenigma kianenigma Apr 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like we could have some kind of test to make sure we don't make this mistake again.

Copy link
Copy Markdown
Member Author

@athei athei Apr 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean having logging enabled during benchmarking by accident?

Copy link
Copy Markdown
Contributor

@kianenigma kianenigma Apr 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but it is probably a PITA

Copy link
Copy Markdown
Member Author

@athei athei Apr 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea on how to construct such a test other than some benchmark which looks for regressions. And something like that would be really nice to have anyways. After each benchmarks of the contracts pallet I need to manually look through a ton of benchmarks and scan for regressions. Would be nice to have this automated.

"libsecp256k1/hmac",
"frame-benchmarking",
"frame-support/runtime-benchmarks",
Expand Down