Skip to content

Commit 0d40573

Browse files
paritytech-release-backport-bot[bot]ggwpezbkontur
authored
[stable2506] Backport #8857 (#9390)
Backport #8857 into `stable2506` from ggwpez. See the [documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md) on how to use this bot. <!-- # To be used by other automation, do not modify: original-pr-number: #${pull_number} --> --------- Co-authored-by: Oliver Tale-Yazdi <[email protected]> Co-authored-by: Branislav Kontur <[email protected]>
1 parent 1acf051 commit 0d40573

File tree

8 files changed

+135
-11
lines changed

8 files changed

+135
-11
lines changed

Cargo.lock

Lines changed: 4 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,7 @@ either = { version = "1.8.1", default-features = false }
783783
emulated-integration-tests-common = { path = "cumulus/parachains/integration-tests/emulated/common", default-features = false }
784784
enumflags2 = { version = "0.7.11" }
785785
enumn = { version = "0.1.13" }
786+
env_filter = { version = "0.1.3" }
786787
env_logger = { version = "0.11.2" }
787788
environmental = { version = "1.1.4", default-features = false }
788789
equivocation-detector = { path = "bridges/relays/equivocation" }

prdoc/pr_8857.prdoc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
title: "[FRAME] Custom log level for the runtime benchmarks"
2+
doc:
3+
- audience: Runtime Dev
4+
description: |-
5+
Changes:
6+
- Add `--runtime-log` option to omni-bencher CLI
7+
- Read env var `RUNTIME_LOG` as fallback to the `--runtime-log` option
8+
- Set custom log level for runtime benchmarks that can be different form CLI level
9+
- Fix issue where old runtimes have a space in the pallet or instance name from breaking change in `quote` macro
10+
crates:
11+
- name: frame-benchmarking-cli
12+
bump: minor

substrate/utils/frame/benchmarking-cli/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ cumulus-client-parachain-inherent.default-features = true
2626
cumulus-client-parachain-inherent.workspace = true
2727
cumulus-primitives-proof-size-hostfunction.default-features = true
2828
cumulus-primitives-proof-size-hostfunction.workspace = true
29+
env_filter = { workspace = true }
2930
frame-benchmarking.default-features = true
3031
frame-benchmarking.workspace = true
3132
frame-storage-access-test-runtime.default-features = true
@@ -86,6 +87,8 @@ sp-io.default-features = true
8687
sp-io.workspace = true
8788
sp-keystore.default-features = true
8889
sp-keystore.workspace = true
90+
sp-runtime-interface.default-features = true
91+
sp-runtime-interface.workspace = true
8992
sp-runtime.default-features = true
9093
sp-runtime.workspace = true
9194
sp-state-machine.default-features = true

substrate/utils/frame/benchmarking-cli/src/pallet/command.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
use super::{
1919
types::{ComponentRange, ComponentRangeMap},
20-
writer, ListOutput, PalletCmd,
20+
writer, ListOutput, PalletCmd, LOG_TARGET,
2121
};
2222
use crate::{
2323
pallet::{types::FetchedCode, GenesisBuilderPolicy},
@@ -50,7 +50,7 @@ use sp_keystore::{testing::MemoryKeystore, KeystoreExt};
5050
use sp_runtime::traits::Hash;
5151
use sp_state_machine::StateMachine;
5252
use sp_trie::{proof_size_extension::ProofSizeExt, recorder::Recorder};
53-
use sp_wasm_interface::HostFunctions;
53+
use sp_wasm_interface::{ExtendedHostFunctions, HostFunctions};
5454
use std::{
5555
borrow::Cow,
5656
collections::{BTreeMap, BTreeSet, HashMap},
@@ -60,11 +60,13 @@ use std::{
6060
time,
6161
};
6262

63-
/// Logging target
64-
const LOG_TARGET: &'static str = "polkadot_sdk_frame::benchmark::pallet";
65-
66-
type SubstrateAndExtraHF<T> =
67-
(sp_io::SubstrateHostFunctions, frame_benchmarking::benchmarking::HostFunctions, T);
63+
type SubstrateAndExtraHF<T> = (
64+
ExtendedHostFunctions<
65+
(sp_io::SubstrateHostFunctions, frame_benchmarking::benchmarking::HostFunctions),
66+
super::logging::logging::HostFunctions,
67+
>,
68+
T,
69+
);
6870
/// How the PoV size of a storage item should be estimated.
6971
#[derive(clap::ValueEnum, Debug, Eq, PartialEq, Clone, Copy)]
7072
pub enum PovEstimationMode {
@@ -252,6 +254,7 @@ impl PalletCmd {
252254
};
253255
return self.output_from_results(&batches)
254256
}
257+
super::logging::init(self.runtime_log.clone());
255258

256259
let state_handler =
257260
self.state_handler_from_cli::<SubstrateAndExtraHF<ExtraHostFunctions>>(chain_spec)?;
@@ -715,7 +718,7 @@ impl PalletCmd {
715718
state: &'a BenchmarkingState<H>,
716719
) -> Result<FetchedCode<'a, BenchmarkingState<H>, H>> {
717720
if let Some(runtime) = self.runtime.as_ref() {
718-
log::info!(target: LOG_TARGET, "Loading WASM from file");
721+
log::debug!(target: LOG_TARGET, "Loading WASM from file {}", runtime.display());
719722
let code = fs::read(runtime).map_err(|e| {
720723
format!(
721724
"Could not load runtime file from path: {}, error: {}",
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// This file is part of Substrate.
2+
3+
// Copyright (C) Parity Technologies (UK) Ltd.
4+
// SPDX-License-Identifier: Apache-2.0
5+
6+
// Licensed under the Apache License, Version 2.0 (the "License");
7+
// you may not use this file except in compliance with the License.
8+
// You may obtain a copy of the License at
9+
//
10+
// http://www.apache.org/licenses/LICENSE-2.0
11+
//
12+
// Unless required by applicable law or agreed to in writing, software
13+
// distributed under the License is distributed on an "AS IS" BASIS,
14+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
// See the License for the specific language governing permissions and
16+
// limitations under the License.
17+
18+
use super::LOG_TARGET;
19+
use sp_core::{LogLevelFilter, RuntimeInterfaceLogLevel};
20+
use sp_runtime_interface::{
21+
pass_by::{PassAs, PassFatPointerAndRead, ReturnAs},
22+
runtime_interface,
23+
};
24+
use std::cell::OnceCell;
25+
26+
thread_local! {
27+
/// Log level filter that the runtime will use.
28+
///
29+
/// Must be initialized by the host before invoking the runtime executor. You may use `init` for
30+
/// this or set it manually. The that can be set are either levels directly or filter like
31+
// `warn,runtime=info`.
32+
pub static RUNTIME_LOG: OnceCell<env_filter::Filter> = OnceCell::new();
33+
}
34+
35+
/// Init runtime logger with the following priority (high to low):
36+
/// - CLI argument
37+
/// - Environment variable
38+
/// - Default logger settings
39+
pub fn init(arg: Option<String>) {
40+
let filter_str = arg.unwrap_or_else(|| {
41+
if let Ok(env) = std::env::var("RUNTIME_LOG") {
42+
env
43+
} else {
44+
log::max_level().to_string()
45+
}
46+
});
47+
48+
let filter = env_filter::Builder::new()
49+
.try_parse(&filter_str)
50+
.expect("Invalid runtime log filter")
51+
.build();
52+
53+
RUNTIME_LOG.with(|cell| {
54+
cell.set(filter).expect("Can be set by host");
55+
log::info!(target: LOG_TARGET, "Initialized runtime log filter to '{}'", filter_str);
56+
});
57+
}
58+
59+
/// Alternative implementation to `sp_runtime_interface::logging::HostFunctions` for benchmarking.
60+
#[runtime_interface]
61+
pub trait Logging {
62+
#[allow(dead_code)]
63+
fn log(
64+
level: PassAs<RuntimeInterfaceLogLevel, u8>,
65+
target: PassFatPointerAndRead<&str>,
66+
message: PassFatPointerAndRead<&[u8]>,
67+
) {
68+
let Ok(message) = core::str::from_utf8(message) else {
69+
log::error!(target: LOG_TARGET, "Runtime tried to log invalid UTF-8 data");
70+
return;
71+
};
72+
73+
let level = log::Level::from(level);
74+
let metadata = log::MetadataBuilder::new().level(level).target(target).build();
75+
76+
if RUNTIME_LOG.with(|filter| filter.get().expect("Must be set by host").enabled(&metadata))
77+
{
78+
log::log!(target: target, level, "{}", message);
79+
}
80+
}
81+
82+
#[allow(dead_code)]
83+
fn max_level() -> ReturnAs<LogLevelFilter, u8> {
84+
RUNTIME_LOG
85+
// .filter() gives us the max level of this filter
86+
.with(|filter| filter.get().expect("Must be set by host").filter())
87+
.into()
88+
}
89+
}

substrate/utils/frame/benchmarking-cli/src/pallet/mod.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
// limitations under the License.
1717

1818
mod command;
19+
mod logging;
1920
mod types;
2021
mod writer;
2122

@@ -28,6 +29,9 @@ use sc_cli::{
2829
};
2930
use std::{fmt::Debug, path::PathBuf};
3031

32+
/// Logging target
33+
const LOG_TARGET: &'static str = "frame::benchmark::pallet";
34+
3135
// Add a more relaxed parsing for pallet names by allowing pallet directory names with `-` to be
3236
// used like crate names with `_`
3337
fn parse_pallet_name(pallet: &str) -> std::result::Result<String, String> {
@@ -187,6 +191,13 @@ pub struct PalletCmd {
187191
#[arg(long, conflicts_with = "chain", required_if_eq("genesis_builder", "runtime"))]
188192
pub runtime: Option<PathBuf>,
189193

194+
/// Set the runtime log level.
195+
///
196+
/// This will overwrite the `RUNTIME_LOG` environment variable. If neither is set, the CLI
197+
/// default set by `RUST_LOG` setting is used.
198+
#[arg(long)]
199+
pub runtime_log: Option<String>,
200+
190201
/// Do not fail if there are unknown but also unused host functions in the runtime.
191202
#[arg(long)]
192203
pub allow_missing_host_functions: bool,

substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,10 @@ pub(crate) fn write_results(
467467
file_name = format!("{}_{}", file_name, instance.to_snake_case());
468468
}
469469
// "mod::pallet_name.rs" becomes "mod_pallet_name.rs".
470-
file_path.push(file_name.replace("::", "_"));
470+
file_name = file_name.replace("::", "_");
471+
// Some old runtimes have a bug with the pallet and instance name containing a space
472+
file_name = file_name.replace(" ", "");
473+
file_path.push(file_name);
471474
file_path.set_extension("rs");
472475
}
473476

0 commit comments

Comments
 (0)