Benchmarking: Add pov_mode to V2 syntax (#3616)

Changes:
- Port the `pov_mode` attribute from the V1 syntax to V2
- Update `pallet-whitelist` and `frame-benchmarking-pallet-pov`

Follow up: also allow this attribute on top-level benchmark modules.

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: command-bot <>

substrate/frame/support/procedural/src/benchmark.rs

@@ -40,10 +40,14 @@ mod keywords {
 	custom_keyword!(benchmarks);
 	custom_keyword!(block);
 	custom_keyword!(extra);
+	custom_keyword!(pov_mode);
 	custom_keyword!(extrinsic_call);
 	custom_keyword!(skip_meta);
 	custom_keyword!(BenchmarkError);
 	custom_keyword!(Result);
+	custom_keyword!(MaxEncodedLen);
+	custom_keyword!(Measured);
+	custom_keyword!(Ignored);
 
 	pub const BENCHMARK_TOKEN: &str = stringify!(benchmark);
 	pub const BENCHMARKS_TOKEN: &str = stringify!(benchmarks);
@@ -73,51 +77,158 @@ struct RangeArgs {
 struct BenchmarkAttrs {
 	skip_meta: bool,
 	extra: bool,
+	pov_mode: Option<PovModeAttr>,
 }
 
 /// Represents a single benchmark option
-enum BenchmarkAttrKeyword {
+enum BenchmarkAttr {
 	Extra,
 	SkipMeta,
+	/// How the PoV should be measured.
+	PoV(PovModeAttr),
 }
 
-impl syn::parse::Parse for BenchmarkAttrKeyword {
+impl syn::parse::Parse for PovModeAttr {
+	fn parse(input: ParseStream) -> Result<Self> {
+		let _pov: keywords::pov_mode = input.parse()?;
+		let _eq: Token![=] = input.parse()?;
+		let root = PovEstimationMode::parse(input)?;
+
+		let mut maybe_content = None;
+		let _ = || -> Result<()> {
+			let content;
+			syn::braced!(content in input);
+			maybe_content = Some(content);
+			Ok(())
+		}();
+
+		let per_key = match maybe_content {
+			Some(content) => {
+				let per_key = Punctuated::<PovModeKeyAttr, Token![,]>::parse_terminated(&content)?;
+				per_key.into_iter().collect()
+			},
+			None => Vec::new(),
+		};
+
+		Ok(Self { root, per_key })
+	}
+}
+
+impl syn::parse::Parse for BenchmarkAttr {
 	fn parse(input: ParseStream) -> Result<Self> {
 		let lookahead = input.lookahead1();
 		if lookahead.peek(keywords::extra) {
 			let _extra: keywords::extra = input.parse()?;
-			return Ok(BenchmarkAttrKeyword::Extra)
+			Ok(BenchmarkAttr::Extra)
 		} else if lookahead.peek(keywords::skip_meta) {
 			let _skip_meta: keywords::skip_meta = input.parse()?;
-			return Ok(BenchmarkAttrKeyword::SkipMeta)
+			Ok(BenchmarkAttr::SkipMeta)
+		} else if lookahead.peek(keywords::pov_mode) {
+			PovModeAttr::parse(input).map(BenchmarkAttr::PoV)
+		} else {
+			Err(lookahead.error())
+		}
+	}
+}
+
+/// A `#[pov_mode = .. { .. }]` attribute.
+#[derive(Debug, Clone)]
+struct PovModeAttr {
+	/// The root mode for this benchmarks.
+	root: PovEstimationMode,
+	/// The pov-mode for a specific key. This overwrites `root` for this key.
+	per_key: Vec<PovModeKeyAttr>,
+}
+
+/// A single key-value pair inside the `{}` of a `#[pov_mode = .. { .. }]` attribute.
+#[derive(Debug, Clone, derive_syn_parse::Parse)]
+struct PovModeKeyAttr {
+	/// A specific storage key for which to set the PoV mode.
+	key: Path,
+	_underscore: Token![:],
+	/// The PoV mode for this key.
+	mode: PovEstimationMode,
+}
+
+/// How the PoV should be estimated.
+#[derive(Debug, Eq, PartialEq, Clone, Copy)]
+pub enum PovEstimationMode {
+	/// Use the maximal encoded length as provided by [`codec::MaxEncodedLen`].
+	MaxEncodedLen,
+	/// Measure the accessed value size in the pallet benchmarking and add some trie overhead.
+	Measured,
+	/// Do not estimate the PoV size for this storage item or benchmark.
+	Ignored,
+}
+
+impl syn::parse::Parse for PovEstimationMode {
+	fn parse(input: ParseStream) -> Result<Self> {
+		let lookahead = input.lookahead1();
+		if lookahead.peek(keywords::MaxEncodedLen) {
+			let _max_encoded_len: keywords::MaxEncodedLen = input.parse()?;
+			return Ok(PovEstimationMode::MaxEncodedLen)
+		} else if lookahead.peek(keywords::Measured) {
+			let _measured: keywords::Measured = input.parse()?;
+			return Ok(PovEstimationMode::Measured)
+		} else if lookahead.peek(keywords::Ignored) {
+			let _ignored: keywords::Ignored = input.parse()?;
+			return Ok(PovEstimationMode::Ignored)
 		} else {
 			return Err(lookahead.error())
 		}
 	}
 }
 
+impl ToString for PovEstimationMode {
+	fn to_string(&self) -> String {
+		match self {
+			PovEstimationMode::MaxEncodedLen => "MaxEncodedLen".into(),
+			PovEstimationMode::Measured => "Measured".into(),
+			PovEstimationMode::Ignored => "Ignored".into(),
+		}
+	}
+}
+
+impl quote::ToTokens for PovEstimationMode {
+	fn to_tokens(&self, tokens: &mut TokenStream2) {
+		match self {
+			PovEstimationMode::MaxEncodedLen => tokens.extend(quote!(MaxEncodedLen)),
+			PovEstimationMode::Measured => tokens.extend(quote!(Measured)),
+			PovEstimationMode::Ignored => tokens.extend(quote!(Ignored)),
+		}
+	}
+}
+
 impl syn::parse::Parse for BenchmarkAttrs {
 	fn parse(input: ParseStream) -> syn::Result<Self> {
 		let mut extra = false;
 		let mut skip_meta = false;
-		let args = Punctuated::<BenchmarkAttrKeyword, Token![,]>::parse_terminated(&input)?;
+		let mut pov_mode = None;
+		let args = Punctuated::<BenchmarkAttr, Token![,]>::parse_terminated(&input)?;
+
 		for arg in args.into_iter() {
 			match arg {
-				BenchmarkAttrKeyword::Extra => {
+				BenchmarkAttr::Extra => {
 					if extra {
 						return Err(input.error("`extra` can only be specified once"))
 					}
 					extra = true;
 				},
-				BenchmarkAttrKeyword::SkipMeta => {
+				BenchmarkAttr::SkipMeta => {
 					if skip_meta {
 						return Err(input.error("`skip_meta` can only be specified once"))
 					}
 					skip_meta = true;
 				},
+				BenchmarkAttr::PoV(mode) => {
+					if pov_mode.is_some() {
+						return Err(input.error("`pov_mode` can only be specified once"))
+					}
+					pov_mode = Some(mode);
+				},
 			}
 		}
-		Ok(BenchmarkAttrs { extra, skip_meta })
+		Ok(BenchmarkAttrs { extra, skip_meta, pov_mode })
 	}
 }
 
@@ -344,6 +455,7 @@ pub fn benchmarks(
 	tokens: TokenStream,
 	instance: bool,
 ) -> syn::Result<TokenStream> {
+	let krate = generate_access_from_frame_or_crate("frame-benchmarking")?;
 	// gather module info
 	let module: ItemMod = syn::parse(tokens)?;
 	let mod_span = module.span();
@@ -364,6 +476,8 @@ pub fn benchmarks(
 	let mut benchmark_names: Vec<Ident> = Vec::new();
 	let mut extra_benchmark_names: Vec<Ident> = Vec::new();
 	let mut skip_meta_benchmark_names: Vec<Ident> = Vec::new();
+	// Map benchmarks to PoV modes.
+	let mut pov_modes = Vec::new();
 
 	let (_brace, mut content) =
 		module.content.ok_or(syn::Error::new(mod_span, "Module cannot be empty!"))?;
@@ -400,6 +514,25 @@ pub fn benchmarks(
 			} else if benchmark_attrs.skip_meta {
 				skip_meta_benchmark_names.push(name.clone());
 			}
+
+			if let Some(mode) = benchmark_attrs.pov_mode {
+				let mut modes = Vec::new();
+				// We cannot expand strings here since it is no-std, but syn does not expand bytes.
+				let name = name.to_string();
+				let m = mode.root.to_string();
+				modes.push(quote!(("ALL".as_bytes().to_vec(), #m.as_bytes().to_vec())));
+
+				for attr in mode.per_key.iter() {
+					// syn always puts spaces in quoted paths:
+					let key = attr.key.clone().into_token_stream().to_string().replace(" ", "");
+					let mode = attr.mode.to_string();
+					modes.push(quote!((#key.as_bytes().to_vec(), #mode.as_bytes().to_vec())));
+				}
+
+				pov_modes.push(
+					quote!((#name.as_bytes().to_vec(), #krate::__private::vec![#(#modes),*])),
+				);
+			}
 		}
 
 		// expand benchmark
@@ -419,7 +552,6 @@ pub fn benchmarks(
 		true => quote!(T: Config<I>, I: 'static),
 	};
 
-	let krate = generate_access_from_frame_or_crate("frame-benchmarking")?;
 	let frame_system = generate_access_from_frame_or_crate("frame-system")?;
 
 	// benchmark name variables
@@ -537,6 +669,16 @@ pub fn benchmarks(
 						];
 						all_names.retain(|x| !extra.contains(x));
 					}
+					let pov_modes:
+						#krate::__private::Vec<(
+							#krate::__private::Vec<u8>,
+							#krate::__private::Vec<(
+								#krate::__private::Vec<u8>,
+								#krate::__private::Vec<u8>
+							)>,
+						)> = #krate::__private::vec![
+						#( #pov_modes ),*
+					];
 					all_names.into_iter().map(|benchmark| {
 						let selected_benchmark = match benchmark {
 							#(#selected_benchmark_mappings),
@@ -544,12 +686,13 @@ pub fn benchmarks(
 							_ => panic!("all benchmarks should be selectable")
 						};
 						let components = <SelectedBenchmark as #krate::BenchmarkingSetup<#type_use_generics>>::components(&selected_benchmark);
+						let name = benchmark.as_bytes().to_vec();
+						let modes = pov_modes.iter().find(|p| p.0 == name).map(|p| p.1.clone());
+
 						#krate::BenchmarkMetadata {
 							name: benchmark.as_bytes().to_vec(),
 							components,
-							// TODO: Not supported by V2 syntax as of yet.
-							// https://github.com/paritytech/substrate/issues/13132
-							pov_modes: #krate::__private::vec![],
+							pov_modes: modes.unwrap_or_default(),
 						}
 					}).collect::<#krate::__private::Vec<_>>()
 				}

substrate/frame/support/test/tests/benchmark_ui/bad_attr_pov_mode_1.rs

@@ -0,0 +1,31 @@
+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use frame_benchmarking::v2::*;
+
+#[benchmarks]
+mod benches {
+	use super::*;
+
+	#[benchmark(pov_mode = Wrong)]
+	fn bench() {
+		#[block]
+		{}
+	}
+}
+
+fn main() {}

substrate/frame/support/test/tests/benchmark_ui/bad_attr_pov_mode_1.stderr

@@ -0,0 +1,5 @@
+error: expected one of: `MaxEncodedLen`, `Measured`, `Ignored`
+  --> tests/benchmark_ui/bad_attr_pov_mode_1.rs:24:25
+   |
+24 |     #[benchmark(pov_mode = Wrong)]
+   |                            ^^^^^

substrate/frame/support/test/tests/benchmark_ui/bad_attr_pov_mode_2.rs

@@ -0,0 +1,33 @@
+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use frame_benchmarking::v2::*;
+
+#[benchmarks]
+mod benches {
+	use super::*;
+
+	#[benchmark(pov_mode = Measured {
+		Key: Wrong
+	})]
+	fn bench() {
+		#[block]
+		{}
+	}
+}
+
+fn main() {}

substrate/frame/support/test/tests/benchmark_ui/bad_attr_pov_mode_2.stderr

@@ -0,0 +1,5 @@
+error: expected one of: `MaxEncodedLen`, `Measured`, `Ignored`
+  --> tests/benchmark_ui/bad_attr_pov_mode_2.rs:25:8
+   |
+25 |         Key: Wrong
+   |              ^^^^^

substrate/frame/support/test/tests/benchmark_ui/bad_attr_pov_mode_3.rs

@@ -0,0 +1,31 @@
+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use frame_benchmarking::v2::*;
+
+#[benchmarks]
+mod benches {
+	use super::*;
+
+	#[benchmark(pov_mode)]
+	fn bench() {
+		#[block]
+		{}
+	}
+}
+
+fn main() {}

substrate/frame/support/test/tests/benchmark_ui/bad_attr_pov_mode_3.stderr

@@ -0,0 +1,5 @@
+error: expected `=`
+  --> tests/benchmark_ui/bad_attr_pov_mode_3.rs:24:22
+   |
+24 |     #[benchmark(pov_mode)]
+   |                         ^

substrate/frame/support/test/tests/benchmark_ui/bad_attr_pov_mode_4.rs

@@ -0,0 +1,31 @@
+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use frame_benchmarking::v2::*;
+
+#[benchmarks]
+mod benches {
+	use super::*;
+
+	#[benchmark(pov_mode =)]
+	fn bench() {
+		#[block]
+		{}
+	}
+}
+
+fn main() {}

substrate/frame/support/test/tests/benchmark_ui/bad_attr_pov_mode_4.stderr

@@ -0,0 +1,5 @@
+error: unexpected end of input, expected one of: `MaxEncodedLen`, `Measured`, `Ignored`
+  --> tests/benchmark_ui/bad_attr_pov_mode_4.rs:24:24
+   |
+24 |     #[benchmark(pov_mode =)]
+   |                           ^

substrate/frame/support/test/tests/benchmark_ui/dup_attr_pov_mode.rs

@@ -0,0 +1,32 @@
+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use frame_benchmarking::v2::*;
+use frame_support_test::Config;
+
+#[benchmarks]
+mod benches {
+	use super::*;
+
+	#[benchmark(pov_mode = Measured, pov_mode = MaxEncodedLen)]
+	fn bench() {
+		#[block]
+		{}
+	}
+}
+
+fn main() {}

substrate/frame/support/test/tests/benchmark_ui/dup_attr_pov_mode.stderr

@@ -0,0 +1,14 @@
+error: unexpected end of input, `pov_mode` can only be specified once
+  --> tests/benchmark_ui/dup_attr_pov_mode.rs:25:59
+   |
+25 |     #[benchmark(pov_mode = Measured, pov_mode = MaxEncodedLen)]
+   |                                                              ^
+
+error: unused import: `frame_support_test::Config`
+  --> tests/benchmark_ui/dup_attr_pov_mode.rs:19:5
+   |
+19 | use frame_support_test::Config;
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D unused-imports` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(unused_imports)]`

substrate/frame/support/test/tests/benchmark_ui/pass/valid_attr_pov_mode.rs

@@ -0,0 +1,74 @@
+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use frame_benchmarking::v2::*;
+use frame_support_test::Config;
+
+#[benchmarks]
+mod benches {
+	use super::*;
+
+	#[benchmark(skip_meta, extra, pov_mode = Measured)]
+	fn bench1() {
+		#[block]
+		{}
+	}
+
+	#[benchmark(pov_mode = Measured, extra, skip_meta)]
+	fn bench2() {
+		#[block]
+		{}
+	}
+
+	#[benchmark(extra, pov_mode = Measured {
+		Pallet: Measured,
+		Pallet::Storage: MaxEncodedLen,
+	}, skip_meta)]
+	fn bench3() {
+		#[block]
+		{}
+	}
+
+	#[benchmark(skip_meta, extra, pov_mode = Measured {
+		Pallet::Storage: MaxEncodedLen,
+		Pallet::StorageSubKey: Measured,
+	})]
+	fn bench4() {
+		#[block]
+		{}
+	}
+
+	#[benchmark(pov_mode = MaxEncodedLen {
+		Pallet::Storage: Measured,
+		Pallet::StorageSubKey: Measured
+	}, extra, skip_meta)]
+	fn bench5() {
+		#[block]
+		{}
+	}
+
+	#[benchmark(pov_mode = MaxEncodedLen {
+		Pallet::Storage: Measured,
+		Pallet::Storage::Nested: Ignored
+	}, extra, skip_meta)]
+	fn bench6() {
+		#[block]
+		{}
+	}
+}
+
+fn main() {}

substrate/frame/support/test/tests/benchmark_ui/pass/valid_basic.rs

@@ -22,7 +22,7 @@ use frame_support_test::Config;
 mod benches {
 	use super::*;
 
-	#[benchmark(skip_meta, extra)]
+	#[benchmark(skip_meta, pov_mode = Measured, extra)]
 	fn bench() {
 		let a = 2 + 2;
 		#[block]

substrate/frame/support/test/tests/benchmark_ui/unrecognized_option.stderr

@@ -1,4 +1,4 @@
-error: expected `extra` or `skip_meta`
+error: expected one of: `extra`, `skip_meta`, `pov_mode`
   --> tests/benchmark_ui/unrecognized_option.rs:26:32
    |
 26 |     #[benchmark(skip_meta, extra, bad)]